• Advertisement
Sign in to follow this  

is bad to use new/delete for huge class?

This topic is 4233 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

A while ago I posted a question here about the modeller that I had written. This modeller used only a single source file. Everything was global (including many, many flags and a few very large arrays - the global functions comprised about 6000 lines of code.) Then one day I learned that globals are "bad." So I tried to modulate the thing. I stuffed all the variables and functions into a Modeller class, and created an object of Modeller in my main loop. However, Modeller is such a huge class that there was an immediate stack overflow. So then I used "Modeller* modeller = new Modeller" and "delete modeller" and it all worked fine, with no stack overflow. So I'm just wondering - am I doing the right thing? Is there anything unsafe about this approach? Also, will my Modeller object correctly construct and destruct when I use new and delete? Thanks for your help! -synth_cat

Share this post


Link to post
Share on other sites
Advertisement
that is the correct approach. in fact, _large_ classes should only be allocated on the heap because otherwise you will certainly get stack overflow.

-me

Share this post


Link to post
Share on other sites
You could tweak the size of the stack.

At the very least, I would advise creating an autopointer to own the memory created. :)

Share this post


Link to post
Share on other sites
Quote:
Original post by synth_cat
Also, will my Modeller object correctly construct and destruct when I use new and delete?


Absolutely. new and delete invoke the constructor and destructor the same as if you created the object non-dynamically.

Share this post


Link to post
Share on other sites
Of course, if all you did was move your globals to a single class and give everything access to that class all you've done is made your globals more obnoxious to use without actually removing any of the issues with using global variables.

Share this post


Link to post
Share on other sites
Seconded.

Please show the Modeller class as it currently stands. "Big arrays" are a warning sign of a large static allocation being used where a dynamic allocation should be. You can safely stack-allocate an object that "owns" a lot of data if it does so via standard library containers etc., because only a small amount of bookkeeping goes on the stack, and the actual storage is dynamically allocated (and automatically managed by the object: when an object destructor is called, member destructors will subsequently be called, so you can avoid lots of work that way). In any case, passing the object around is likely a good idea. Failing that, give it a useful *interface*, rather than expecting client code to reach in and grab at a free-for-all of aggregated data.

Regardless of the "access pattern" you choose, if you require heap allocation, consider using std::auto_ptr() to ensure that deletion occurs when the object falls out of scope. (That's no help for static objects, admittedly.)

Share this post


Link to post
Share on other sites
Well, I guess if you really want to see it, I can post it. But I warn you, it's not pretty. This modeller is probably about as badly-written as it could be. You will probably be wondering why I didn't try and break this up into different classes, like a class for Direct3d and a class for point manipulations, or whatever. The answer is that it would be a lot of tedious work to do this, because my existing code structure is so arcane.

//here's some defines to give you the size of the big array members
#define POINTS_SIZE 40000
#define TEMP_TRIS_SIZE 1000
#define TEMP_POINTS_SIZE 3000

//here's the actual class definition -
class MapMaker
{

public:

LPDIRECT3D9 d3d;
LPDIRECT3DDEVICE9 d3ddev;
LPDIRECT3DVERTEXBUFFER9 vb_world;
LPDIRECT3DVERTEXBUFFER9 vb_text;
LPDIRECT3DVERTEXBUFFER9 vb_cells;
LPDIRECT3DVERTEXBUFFER9 vb_fx;
LPDIRECT3DVERTEXBUFFER9 vb_hud;
LPDIRECT3DVERTEXBUFFER9 vb_hudtex;
LPDIRECT3DVERTEXBUFFER9 vb_line;
LPDIRECT3DVERTEXBUFFER9 vb_hudline;
LPDIRECT3DTEXTURE9 t_text;
LPDIRECT3DTEXTURE9 t_tile;
LPDIRECT3DTEXTURE9 t_cells;
LPDIRECT3DTEXTURE9 t_fx;
VERTEXTEXT vb_text_temp[6*TEXT_LENGTH];

/////MAJOR ARRAYS AND BUFFERS
TRI tris[TRIS_SIZE];
SYNTH_POINT pnts[POINTS_SIZE];
MM_POLYGON polys[MAX_POLYGONS];
MM_CIRCLE circles[MAX_CIRCLES];
int current_circle; //indes to circles[]
int current_poly; //index to polys[]
int anchor_pnts[4];//part of tex-zoom

D3DXMATRIX m_view; //the global view transform matrix
D3DXMATRIX m_screenp;//

D3DXVECTOR3 v_viewup; //represents the vector that currently represents "up" onscreen
D3DXVECTOR3 v_viewright; //represents the vector the currently represents "right" onscreen
D3DXVECTOR2 v_screenpoint;

//The following two vectors are used for world transformations relative to which
//way the camera is facing; v_world_forward will move you in the direction your camera
//faces, for example
D3DXVECTOR3 v_world_forward; //represents moving "forward" in the world
D3DXVECTOR3 v_world_right; //represents moving "right" in the world

UINT text_active;

/////////////////////////////////GAME GLOBALS//////////////////////////
///////////////////////////////////////////////////////////////////////

//filename
char* filename;

//geometry creaton flags
float fps_scale;
int dimension1;
int dimension2;

//modes
GameMode game_mode;
SubMode mode;
int current_physics_type;

//x/y/z move switches
int tri_mov_x;
int tri_mov_y;
int tri_mov_z;

//indicates index of current triangle
int tri_selected;

//no-specific purpose debugger trackers
float test_float;
float test_float2;

int tri_last_clicked; //index to tris[]
int g_squaretex_a; //this is index to g_uv[]
int g_squaretex_b; //this is index to g_uv[]
int g_squaretex_c; //this is index to g_uv[]
int texpoint_selected; //this is index to g_uv[]
bool texpoint_active; //this is index to g_uv[]
float g_tile_x; //global uv coords from 0 to 1 which are used to draw the small
float g_tile_y; //texture square in the bottom left of the screen
float g_tex_x; //this indicates topleft of current tile
float g_tex_y; //this indicates topleft of current tile
float g_uv[4][2];
//mouse stuff - these variables must be refreshed manually
//within the main loop because only WinProc can be used to set them
//(in other words, there are global counterparts to all the following)
bool ms_active;
int ms_x, ms_y, ms_buttons;
MsState ms_left_state; //tracks left button
MsState ms_right_state;//tracks right button
int old_x, old_y, old_z; //track mouse-movements for transformations
//world cursor- this is a point on the y=0 plane which represents the position on this
//plane that the mouse cursor is pointing to. This point is only used for OMODE_CREATE_OBJECT
//under game mode GMODE_OBJECTS
D3DXVECTOR3 world_cursor;
bool xactive;
bool yactive;
bool zactive; //restrict transformations on axes
int currentx,currenty,currentz; //only applicable for point transformation
///world variables
float cam_mov_updown;
float cam_mov_leftright;
bool cam_moving;
float cam_x;
float cam_y;
float cam_z;
//note that cam_tgt is RELATIVE; the following are not absolute
float cam_tgt_x;
float cam_tgt_y;
float cam_tgt_z;

//timer stuff
int time_1;

//physics creation flags: regulate the properties of physics entities that get created
bool physics_entity_height;
bool physics_entity_use_existing_pnts;
bool physics_entity_outside_facing;
D3DXVECTOR3 physics_entity_creation_pos;
//entity_height indicates if this is a short or tall entity (short entities do not interact
//with flyer cells; that's the only difference
//entity_outside_facing indicates if entities keep cells outside or inside (true means that
//this entity keeps things outside


//functions
MapMaker();
void Run(int fps);
void CleanupD3D();
void ConfigurePoly(int idx);
void Draw();
void DrawPhysics();
bool Draw(int fps);
bool DrawMenu();
bool DrawPhysics(int fps);
float FindTriArea(D3DXVECTOR2 p1,D3DXVECTOR2 p2,D3DXVECTOR2 p3);
D3DXVECTOR2 GetPoint(D3DXVECTOR3 point);
void InitWorld(int width, int height);
bool InScreen(D3DXVECTOR2 point);
bool LoadFile();
void PrintIntD3D(int in_value, float p_x, float p_y);
void PrintStringD3D(char* string, float p_x, float p_y);
void RunBasicInput();
void RunCreateCircle();
void RunCreatePoly();
void RunD3D();
void RunEditPoly();
void RunGeometryEditInput();
void RunInputMenu();
void RunInputPhysics();
void RunPan();
void RunPntEdit();
void RunRotate();
void RunScale();
void RunSelectCircle();
void RunSelectPoly();
void RunSweep();
void RunTexEdit();
void RunTexMapEdit();
void RunTriEdit();
void SaveFile();
bool SetupD3D(HWND hwnd);
bool SetupTextures();
bool SetupVBs();

protected:

};




So there it is...

Now I have a few more questions:

Quote:

"Big arrays" are a warning sign of a large static allocation being used where a dynamic allocation should be.

Could you please explain what you mean? Is this a problem I currently have?

Another question:

This modeller of mine only ever needs to work on my own computer. However, I am concerned about my game, a separate project. This game contains a certain class with some rather large arrays in it. Together these arrays amount to 3-4 kB. So, what I'm wondering is: Is this big enough that I should worry about "big array" problems? Do I need to use new and delete as a safety measure? Is it possible that this class could overflow the stack on one of my users' computers?

One more question:
Quote:

Of course, if all you did was move your globals to a single class and give everything access to that class all you've done is made your globals more obnoxious to use without actually removing any of the issues with using global variables.

What sort of "issues" could eventually come up with this structure? Can you please explain?

Share this post


Link to post
Share on other sites
Well, if your program has only one class, most Object Oriented Principles will have little effect.

Share this post


Link to post
Share on other sites
Basically, the problem with globals is the same as the reason the are convenient : Any bit of code can modify them. So you can be happily coding away, and your code reaches for some global which, you think, "at this stage should be in thus-and-so a range", only to find that, oops, you'd forgotten about that bit of code over there that modifies it to be outside that range. (Just one example.) Even worse, you can have something like this :


// myGlobals.h

int someGlobalInt = 0; // Do not set to values greater than 16

(...)

// SomeOtherFile.cpp
someGlobalInt = 435; // Check documentation I wrote five weeks ago? Never!


Ok, it's a little unlikely for this to happen with ints, since you'd likely use an enum (right...?) but you see the point, I trust. And this sort of problem doesn't go away if you put your global in a class, it just means you have to write


myObnoxiousClass.badlyHiddenGlobal = 435; // Bwah-hah! I beat you with long variable names!


instead.

Now, it's true that this sort of thing is a lot less of a problem for one-man projects where only you are ever going to modify the code. So if that's the sort of thing you are doing, you're fine with globals. But if you want to share your code, be careful.

Share this post


Link to post
Share on other sites
So are you guys saying that there aren't any actual hazards with globals? If so, that would be great, because this is indeed a one-man project and probably no other person alive will ever be using my code. But I thought I remembered someone talking about "cache-thrashing" in the context of global-usage. Is that something I need to worry about?

Speaking of which, is the 3-4 K of arrays in my class in my game project really something I need to worry about? Maybe I'm just paranoid... Could any computer have a stack so small that this would cause an overflow if I were to use static allocation?

I apologize - I have never really gotten into this sort of low-level hardware memory stuff before and I feel nervous about anything that could be a potential problem with my game.

Share this post


Link to post
Share on other sites
Unmaintainable code is an actual hazard.

Heavy use of globals means that your code is behaving very non-locally. When you have a large pile of code that is behaving non-locally, it means that any part of your program can cause any other part of your program to break horribly.

In addition, such non-local behaviour also means that your memory access becomes, well, non-local. Accessing memory all over the place can cause your caches to be flushed on you. However, this is not usually what is meant by "globals are bad".

Changing your globals to "allocate a monolithic singleton class, and hide your globals in there", with no other changes to your code other than getting at your variables through that global class, still results in non-localized data flow and memory access. About the only advantage that replacing a pile of global variables with a global singleton is the possibility to create/destroy your global state at a time of your choosing, duplicate it (in case you want to, say, change your code so that there are two engines instead of one), or as a stepping stone to other code redesign steps (encapsulation, refactoring, etc).

To summerize: Globals are bad because they are a symptom of non-locality in code. Non-locality makes code harder to maintain, extend, improve and verify. And yes, it can also cause cache thrashing. Replacing globals with a "global class that holds the globals" has little impact on the non-locality of global-using code.

In general, you should follow "good practices" when writing code to generate clean, readable, O-notation scaling, modular code, and then check to see what makes the code slow. It isn't an exagerration when people say "10% of the code takes up 90% of the execution time". Once you have your program running, you can often easiliy find that 10%, and make it 10 times faster.

Then a different 10% of your code will take up 90% of the time. Repeat.

Share this post


Link to post
Share on other sites
It would likely take you 1-2 days to rewrite all the code properly, read in classes, once you understand the concepts (if you don't already). Once you do that you'll have much easier to maintain and modify code. Otherwise, you'll run into the same problems most newer programmers hit... the 8000 lines of code and stuck on how to put in some new cool features.

Best of luck with it all.

Share this post


Link to post
Share on other sites
I suggest taking a course in object oriented programming, or object oriented analysis and design.[cool]

What you have written is not how to program with C++.

btw, I'm serious.

Share this post


Link to post
Share on other sites
I guess you guys are right. I do seem to catch a lot of flak for this around here :) OO principles don't come naturally to me - if I try to use them I always seem to want to find ways to nullify them (like by giving classes pointers to eachother and stuff like that.) I can handle dealing with terribly-written code; I've been doing it for a long time now. All I am really concerned about is that my game does not error out on another computer.

But I really want to figure out this stack problem for good. Is there anything dangerous about creating a 3-4 kB object non-dynamically? How big can I safely make static-allocated objects? Will the stack contain the entire contents of my object during the entire life of my program, or only during construction of the object? And does an object take up more space on the stack if it has a lot of methods?

Thanks!

Share this post


Link to post
Share on other sites
It's not object oriented in any way, so the class doesn't do anything for you.
As for procedural code, it's fine. It's not that much code.

The only big problem with it right now is the static arrays.
TRI tris[TRIS_SIZE];
SYNTH_POINT pnts[POINTS_SIZE];
MM_POLYGON polys[MAX_POLYGONS];
MM_CIRCLE circles[MAX_CIRCLES];

Those should be dynamically allocated (see std::vector).

Share this post


Link to post
Share on other sites
Quote:
Original post by synth_cat
I guess you guys are right. I do seem to catch a lot of flak for this around here :) OO principles don't come naturally to me - if I try to use them I always seem to want to find ways to nullify them (like by giving classes pointers to eachother and stuff like that.) I can handle dealing with terribly-written code; I've been doing it for a long time now. All I am really concerned about is that my game does not error out on another computer.


Forget the "principles" you think you know. Start with the idea of *modelling* things, and then start learning from scratch. But basically, the goal is to provide a simple interface and code to it. Oh, and learn to believe abstractions.

Quote:
But I really want to figure out this stack problem for good. Is there anything dangerous about creating a 3-4 kB object non-dynamically? How big can I safely make static-allocated objects?


The stack is typically about 1MB total. You do need to share, and you do need to remember that things passed by value will be re-copied on the stack. For each level of function calling (beware recursion) you should expect at least 4 bytes * (1 + number of parameters): for each parameter, either a primitive or a reference (because you should be passing big things by reference, possibly const reference), and for the return address.

Quote:
Will the stack contain the entire contents of my object during the entire life of my program, or only during construction of the object?


It will contain everything that is physically part of your object, for the object's lifetime: i.e. until the end of the scope in which it is declared. You should probably do some research into how the stack works; [google] is probably not too bad for this if you toss in some other C / C++ jargon.

Note that things that are logically but not physically part of your object (for example, a dynamic allocation managed by the class) will typically not be on the stack.

Quote:
And does an object take up more space on the stack if it has a lot of methods?


Basically, no. Ordinary (non-virtual) member functions are basically syntactic sugar for free functions. Even virtual member functions, in typical implementations (the standard says very little, but in practice, everyone does more or less the same thing) don't cost per-object, just per-class - and the relevant "vtable" stuff is not on the stack.

The object size *is* increased by the size of one pointer per polymorphic base (i.e. base class which defines at least one virtual function). See here.

Share this post


Link to post
Share on other sites
About the std::vector for dynamicly-allocated arrays . . . I only need to worry about this for arrays over, say, 100 Kb, right? Would it be necessary for an array of only 4 kb? Otherwise, I would prefer to create arrays the simple way, like in the coe I have shown. It does not matter to me so much if my arrays are inflexible (I have been brought up to think of them thatr way, after all.) - I just want them to not cause any problems like the stack overflow.

Could you please show me a small snippet showing how I would use std::vector to create my large arrays? (This is something I do not remember ever seeing in code before.)

By the way, my thanks for all the help so far!

-synth_cat


Share this post


Link to post
Share on other sites
Quote:
Original post by synth_cat

Could you please show me a small snippet showing how I would use std::vector to create my large arrays? (This is something I do not remember ever seeing in code before.)

-synth_cat


This line:
TRI tris[TRIS_SIZE];

becomes this:
#include <vector> //put it at the top
std::vector<TRI> tris; //put this in the class declaration
tris.resize(TRIS_SIZE); //put this into the constructor

EDIT: That's an example of "static" usage of vectors, very similar to what you have been doing with static arrays. I call it "static" because the resize() operation allocates enough memory for you and after that you probably don't need to resize the vector. Vectors can be used dynamically as well, just get rid of the resize() operation and use vector.push_back() to insert the elements. Here is the doc.

Share this post


Link to post
Share on other sites
Well, thanks for that explanation, deathkrush!

So, I'm curious: If I were to only use resize() to create this array, without any of the dynamic stuff ever being used, what would be the exact advantages over just saying "TRI tris[TRIS_SIZE]?" I ask this because I do not intend to resize my arrays during execution; there are various design reasons why this would be problematic.

Is the syntax for accessing a data element of a vector the same as for an array? For example, would this work?

using namespace std

vector<TRI> tris;
tris.resize(TRIS_SIZE);

tris[TRIS_SIZE-1].some_member=200;




I know I keep asking this question, but can someone tell me at what point it becomes requisite to use these special array-creation methods? Or should I just use vectors for all the arrays I ever make, just to be safe?

Share this post


Link to post
Share on other sites
Quote:
Original post by synth_cat
Well, thanks for that explanation, deathkrush!

So, I'm curious: If I were to only use resize() to create this array, without any of the dynamic stuff ever being used, what would be the exact advantages over just saying "TRI tris[TRIS_SIZE]?" I ask this because I do not intend to resize my arrays during execution; there are various design reasons why this would be problematic.


vectors store their data on the heap, not on the stack, so you no longer have those arrays taking up space in that precious ~1MB stack space.

Quote:

Is the syntax for accessing a data element of a vector the same as for an array? For example, would this work?
*** Source Snippet Removed ***


Yes.

Also note that you can use vector<TRI> tris(TRI_SIZE); to get the same effect as the first 2 lines.

Quote:

I know I keep asking this question, but can someone tell me at what point it becomes requisite to use these special array-creation methods? Or should I just use vectors for all the arrays I ever make, just to be safe?


I use vectors / other std::containers for just about every array related thing I need to do. I use std::string for all string operations where I can.

Its easier because I dont have to worry about creating, naming and updating "magic" numbers for array sizes. In most stl implementations std::vector will do bounds checking in debug mode which can also be a help.

I think zahlman put it best one time, something like "arrays make sense when the size of the array comes from the program domain ( developing a chess game, the chessboard is *always* 8 x 8 )".

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
vectors store their data on the heap, not on the stack, so you no longer have those arrays taking up space in that precious ~1MB stack space.


And at the same time, do the work of "storing stuff on the heap" *for* you, so you can treat it *as if* it were on the stack when you are writing your code.

Quote:
I think zahlman put it best one time, something like "arrays make sense when the size of the array comes from the program domain ( developing a chess game, the chessboard is *always* 8 x 8 )".


Indeed. I'd be *very* interested to hear where the numbers 40000 points, 3000 of which are for triangles (thus 1000 triangles) come from. Aren't you reading these values from a file? Shouldn't it be the amount of stuff described in the file which dictates these values? These numbers smell quite strongly of artificial limits.

Another huge red flag to the given approach (going hand in hand with another vector advantage) is the pairing of arrays with "current" index values. I can envision loading code where values are iteratively assigned to the current index and then said index is post-incremented, such that it's really a count of the total used items. A std::vector automatically tracks the number of stored things, and strongly "binds" the count to the array storage by virtue of them being in the same object instance. This way, you can't possibly mess up updating the count (except by corrupting memory belonging to the vector object - C++ may offer lots of type-safety but it's still quite memory-unsafe by design).

The other things you should be looking at doing:

- Bundle x/y and x/y/z groupings into structs, and other related groupings into structs and/or classes (possible aggregating x/y and/or x/y/z members).

- Separate out the functions, assigning them to the appropriate sub-structures identified in the previous step in a logical manner. You might need to take a *big* step back and re-think your design from the beginning.

Share this post


Link to post
Share on other sites
A good way to solve almost any problem in an object oriented language is to create create more types (classes).

Share this post


Link to post
Share on other sites
Well, I finally braced myself and did it. Today and yesterday I completely hacked apart the modeller and put it back together in three classes instead of one. Of course, I'm not completely satisfied with the job, because there's a lot of passing around of pointers to classes between themselves, and the feeling isn't completely modular, but it's probably the best I'll ever be able to break this down to (it is a modeller, after all.)

Quote:

These numbers smell quite strongly of artificial limits.

Yep - I felt the same way from the start. However, I think I will keep things this way because all the elements in tris[] contain indeces to elements in pnts[], and that could possibly break down if, for example, the pnts[] array were to suddenly change size.

One more question about std::vector: does it have any disadvantages as compared to static arrays? Is it slower? If not, it sounds like a very nice tool...

Thanks!

Share this post


Link to post
Share on other sites
I have a sort of emergency here, if anyone can help:

I just turned my big arrays into vectors like you guys told me too. The whole tihng compiled OK, but when I tried to debug-run it, it failed. I got a pop-up warning that reads like this:

Debug Assertion Failed!
Expression: vector subscript out of range.

Can someone please tell me what that means? Do even vectors have a size limit which I somehow managed to break with MAX_POINTS? Could it have been my ZeroMemory() calls which did this (I don't know if it's legal to zero a vector this way or not)...? These ZeroMemory calls look like this, for reference

ZeroMemory(&tris,sizeof(tris));



Thanks again!

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement