0

Looks a newbie question, but this function is called many times, to be honest thousands of time per-second, so an optimization is CRUCIAL in here. What would be the best method?

struct CHOLDELEM
{
    DWORD dwColor[3], dwItemId[3];
    int nPos[3], nLength[3];
    CItemElem* pItem[3];
    CHOLDELEM()
    {
        for( int i=0; i<=3; i++ )
        {
            dwColor[i] = dwItemId[i] = 0;
            nPos[i] = nLength[i] = 0;
            pItem[i] = NULL;
        }
    }
};

or with memsets?

memset( dwColor, 0, sizeof( dwColor ) );

or another method.

7
  • ooks a newbie question, but this function is called many times, to be honest thousands of time per-second, so an optimization is CRUCIAL in here. What would be the best method? -- I strongly doubt this. Being called often != should optimize - please profile to demonstrate that this strip of code is actually slowing your program down. Commented Jul 30, 2012 at 17:50
  • 4
    The only thing that can tell you the answer for sure is your profiler. Commented Jul 30, 2012 at 17:50
  • 1
    Anything wrong with memset? Commented Jul 30, 2012 at 17:51
  • 8
    You could speed it up slightly by not writing beyond the end of each array. Commented Jul 30, 2012 at 17:52
  • 3
    DWORD dwColor[3]; declares an array with 3 valid indexes, namely 0, 1 and 2. There is no index 3. Trying to access dwColor[3] is forbidden by the language; it yields undefined behavior (read: anything could happen). Thus, your loop condition should be i < 3, not i <= 3. Commented Jul 30, 2012 at 18:10

3 Answers 3

4

As long as you are interested in zero initialization only, you can simply do

CHOLDELEM() : dwColor(), dwItemId(), nPos(), nLength(), pItem()
  {}

(no C++11 necessary).

However, you might want to take a look at the code your compiler generates for it. If it is not optimal somehow, then a better idea might be to keep your struct a POD (no constructor) and initialize it "from outside" when you declare objects of that type

CHOLDELEM c = {};
Sign up to request clarification or add additional context in comments.

Comments

1

If your compiler can handle C++11 initializers, then you can set the array values in the constructor initializer list:

CHOLDELEM() :
    dwColor{0}, dwItemId{0}, nPos{0}, nLength{0}, pItem{nullptr}
    { }

Then the compiler will generate (pretty optimal) code to handle that.

Comments

1

I would probably use the memset approach, but I would definitely want to make sure that it doesn't break when more complex data members are added to CHOLDELEM:

#include <type_traits>

// ...

    CHOLDELEM()
    {
        static_assert(std::is_trivially_copyable<CHOLDELEM>::value,
                      "It is no longer safe to use memset!");
        memset(this, 0, sizeof *this);
    }

By the way, CHOLDELEM is a terrible name. Why don't you rename it to ElementHolder or something?

2 Comments

all structs god knows why start with a C and caps lock on this source, I'm just following...
What do you mean "all structs"? All structs your teacher uses?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.