   profiling discussion: STL vs a makeshift approach
  I spent disproportionate four hours last night with hopes of improving the rather crude DIY memory management scheme for a bit of code that I have. I've been profiling it back and forth for a couple of hours now and I have to conclude: it's either VS, me or it's STL, but something is definitely a bit unexpected here.The memory management scheme is for an object pool system that handles dynamic object emission. I set the test cases up so that the STL version (I'm using std::vector) reserves more than the required amount of elements and just emits pointers to them when an element is needed (the allocation is not profiled). The DIY version is a crude non-STL based link system that allocates a new element as needed. I profiled a variety of smaller segments and the routine as a whole and the results seem to be very consistent: the STL version is slower by a factor of two. This happens across all segments except the one where the DIY version allocates new elements, which perform more or less equally.The main difference between the two versions is how elements are accessed. Since I'm dealing with polygon data, much of the access is circular, requiring some form of modulus to be used pretty heavily (I replaced this with the hack 'mod = (n & (size - 1))' ) and it really did have only a marginal impact. The rest of the code differs largely only by indexed vs pointer-based element access. This seems to generate way more overhead for the STL version than I'd expected. Here are two versions of the same bit of code (a very simple sample excerpt only):The STL version for(int i = 0; i < (int)f0->vertices.size(); i++){f0->vertices->real = f1->plane.GetPointDistance(f0->vertices->vertex->vPosition);if(f0->vertices->real < -EPSILON)iSideA++;else if(f0->vertices->real > EPSILON)iSideB++;}and the DIY version:IVertexRef* v = f0->vertices;while(v){v->real = f1->plane.GetPointDistance(v->vertex->vPosition);if(v->real < -EPSILON)iSideA++;else if(v->real > EPSILON)iSideB++;v = v->next;}The two clock in with a difference of magnitude of about two to three which (as mentioned above) remains constant across all of the code involved.TBH I'm slightly confused - I figured my DYI approach with stress on coding speed would be blown out of the water once I got caching working. But this most definitely doesn't seem to be the case.I'm not sure which approach I should favor here as the test cases I've tried are by no means comprehensive (they're very simple, really). If anyone has more experience with this, don't be shy to voice your opinion :)Using: VS2008, Win 7 64bitPS - enabling optimization (full or partial) in VS seems to mess up a very important 'this' pointer. That doesn't happen when I disable optimization or compile in debug mode. I'm assuming it's a bug since it happens almost immediately after program entry (eg I can't think of anything that could mess up memory from elsewhere; if I trace it, I can enter the main start-up function (not the constructor of the class) and when first accessing/initializing a member object of the class, 'this' pointer becomes corrupt). Quick googling seems to confirm that it's an optimization thing, but I can't seem to find a fix for it. Looks like I very serious issue TBH.   
Quote:PS - enabling optimization (full or partial) in VS seems to mess up a very importantthispointer. That doesn't happen when I disable optimization or compile in debug mode. I'm assuming it's a bug since it happens almost immediately after program entry (eg I can't think of anything that could mess up memory from elsewhere; if I trace it, I can enter the main start-up function (not the constructor of the class) and when first accessing/initializing a member object of the class,thispointer becomes corrupt). Quick googling seems to confirm that it's an optimization thing, but I can't seem to find a fix for it. Looks like I very serious issue TBH.Post the code from main where the crash comes from, a corrupt "this" usually comes from optimizing away the debug safeguards.  So you have a bad object, you call a member function (or constructor), and the function accesses a bad "this".  Could be something on the stack, memory overrun, or even just an uninitialized pointer that you attempt to deference.Quote: for(int i = 0; i < (int)f0->vertices.size(); i++){f0->vertices->real = f1->plane.GetPointDistance(f0->vertices->vertex->vPosition);if(f0->vertices->real < -EPSILON)iSideA++;else if(f0->vertices->real > EPSILON)iSideB++;You are doing a lot more work here than in your other version. The optimizer might be able to cut some of the work out, but you can cut a lot out too, and clean up the look of the code.For instance, replace all the duplicated code by doing this at the top of the loop:const IVertexRef *vertIter = f0->vertices;const float real = vertIter->real;int the for loop, you should be doingfor( std::vector< blah >::size_type i = 0; i < f0->vertices.size(); ++i )casting the vector size to an int is VERY BAD.Using an iterator would be even better though.Is there a reason you have pointers to vertices? instead of storing vertices themselves in the std::vector?  You could just be running into issues because of the random memory access (computers like it better when you access everything in order instead of jumping around);  Vector access will be slow unless you:#define _SECURE_SCL 0#define _HAS_ITERATOR_DEBUGGING 0And then you build in fully optimized mode.A corrupt 'this' pointer in optimization mode means you forgot to set some pointer to a proper memory address.  You're not getting the same issue in debug mode because debug mode sets all uninitialized pointers to some fail-safe value.   ;  Here you go://staticIEngine* IGameEngine::Launch(){GUARD();if(!IEngine::engine){IEngine::engine = new IGameEngine;QASSERT(IEngine::engine);IEngine::engine->Initialize(); //from hereIEngine::MainLoop();}RETURN(IEngine::engine);}//to hereWSEERROR IGameEngine::Initialize(){GUARD();   //this pointer becomes corrupt herehm = new IHeightmap; //<-QASSERT(hm);EASSERT(hm->Allocate(70, 40));....RETURN(WS_E_NOERROR);}'hm' is pre-set to NULL in the constructor. When tracing, the this pointer changes from 0x006017d3 to 0x9e07cbf4 when reaching the indicated line.Quote:You are doing a lot more work here than in your other version. The optimizer might be able to cut some of the work out, but you can cut a lot out too, and clean up the look of the code.For instance, replace all the duplicated code by doing this at the top of the loop:const IVertexRef *vertIter = f0->vertices;const float real = vertIter->real;int the for loop, you should be doingfor( std::vector< blah >::size_type i = 0; i < f0->vertices.size(); ++i )casting the vector size to an int is VERY BAD.I know and that's why I was trying to do a fully optimized build :). IMO that still shouldn't account for doubling the execution time (apparently it does, though).PS - Why precisely is casting the size to an int so bad?Quote:Is there a reason you have pointers to vertices? instead of storing vertices themselves in the std::vector? You could just be running into issues because of the random memory access (computers like it better when you access everything in order instead of jumping around)I know - that's why I'm implementing a cache. I'm using pointers because I need to use a few bits of additional information with my vertices and it's easier to package that to an intermediate structure. I can use the pointers themselves for fast comparison.Quote:#define _SECURE_SCL 0#define _HAS_ITERATOR_DEBUGGING 0And then you build in fully optimized mode.Thanks for the hint - I wasn't aware of this! Once I find the this pointer morphing culprit I'll give it a try.Quote:A corrupt 'this' pointer in optimization mode means you forgot to set some pointer to a proper memory address. You're not getting the same issue in debug mode because debug mode sets all uninitialized pointers to some fail-safe value.I know that as well. I'm scouring the code for a possible issue, but I should mention that release mode without optimization compiles and runs just fine.   ;  1) sorry, check my edits above for more hints.2) whats GUARD()3)are you sure "this" went corrupt? in release the debugger isn't always good at actually keeping track of everything like it does in debug. There is a very good chance "this" is still ok, but the watch is telling you the contence of a register it THOUGHT this was in, but it isn't because of the functioncall to new.when "this" is good, drag the address in the watch, and cast it. make sure all the members look good, then step. if all the members still look good, then "this" could actually still be ok, and your object could still be ok. (ie keep track of the "IEngine::engine" pointer, and make sure that it isn't getting trashed)4)what does the IHightMap constructor do? could it be trashing memory? does it crash before or after that constructor?i love edits!Quote:I know and that's why I was trying to do a fully optimized build :). IMO that still shouldn't account for doubling the execution time (apparently it does, though).all those [] operators are "checked". You're doing a TONNE more work because that operator has lots of debug code in it.Quote:PS - Why precisely is casting the size to an int so bad?size isn't an int, it could be smaller, but is likely the same size or bigger. But in the grand scheme of things, you are begging for integer overflow bugs because you cast an unsigned to a signed value.  It isn't a good habit to get into, and it is important to watch for signed/unsigned bugs.For instance:(int)(std::vector::size() - 1)is completely different from( (int)std::vector::size() ) - 1and it is hard to just see that, because you casted away the proper size and type of your iterator with (int).;Quote:Original post by nullsquared#define _SECURE_SCL 0#define _HAS_ITERATOR_DEBUGGING 0^^ QFE -- MSVC's STL containers contain a lot of debugging code unless you do this (even in release builds).How are you determining that this is corrupt? Is the program misbehaving, or is it just that the debugger/watch windows aren't correct? If it's the latter, that's not a problem, it's just that the optimised assembly is too chopped to pieces for the debugger to understand fully.Also, calling size every iteration can be a minor slow-down, instead you can write:for( uint i = 0, s = f0->vertices.size(); i != s; ++i );  I love edits, too :)2) GUARD() is a stack guard macro. The code behaves the same with and without it. I've commented it out to make things a bit more clear.3) That's precisely what I'm doing. Apparently the pointer becomes corrupt (now it seems to change to 0x00000030) just before entering IHeightmap::IHeightmap (eg the constructor is never run). At that point all contents of 'this' (IGameEngine) and its base class (IEngine) become corrupt (they also turn red in the watch).4) It used to run the default constructor, but I provided it with a custom one and it changed nothing.>> all those [] operators are "checked". You're doing a TONNE more work because that operator has lots of debug code in it.I kinda hoped they wouldn't be in release mode with everything disabled.>> size isn't an int, it could be smaller, but is likely the same size or bigger. But in the grand scheme of things, you are begging for integer overflow bugs because you cast an unsigned to a signed value. It isn't a good habit to get into, and it is important to watch for signed/unsigned bugs.My logic was that I'd need to be approaching half the int range for there to be any real danger of things going wrong. Since that is practically impossible in my application and would otherwise crash the program anyway if it were to happen, I resorted to a simple cast. There's no harm in changing the iterator to unsigned, but I really do love my clarity in most cases (eg simplest possible readable code).   ;  Generally bad this pointers come from code that looks something like this. While it will crash in the member function that's not where the bug is.thing = NULL; // Or any other invalid pointerthing->SomeMemberFunction();Assuming you're compiling a 32-bit exe then the this pointer will be passed to functions in the ecx register. The optimizer will reuse the register for other things though if it can. Check the value in ecx on the first line of the function to see if it's been passed in incorrectly.If that's not the problem could you post the mixed source / disassembly for the function that fails (right click, select go to disassembly in the debugger). The register values at the time of the crash might be handy too.   ;  Okay, vat ze hell... The following code does not execute:int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow){IHeightmap* hm = NULL;hm = new IHeightmap; //<--   return 0;}When tracing to the indicated line, the symbol hm is not found.I'm not sure how to proceed from here.Edit: when running the program, it doesn't crash or anything - it just falls through.Edit 2: disabling the debug database (in release mode) seems to fix that. However, even stranger behavior ensues - a relatively random loop does not exit normally, but finishes properly if I add a debug output line to it. I've seen this kind of before and it's always been a memory corruption issue. Is there any way to debug this more effectively than starting to comment out code line by line (yes, I already know an answer to that...)?[Edited by - irreversible on April 19, 2010 7:21:25 AM];  hm is possibly optimized away. Introduce side-effects.   