CODEWITHSUNDEEP
c++windowsvisual-studiodictionarystd
c00000fd
c00000fd

Reputation: 22275

How to optimize std::map insert() function?

The best way to explain what I'm trying is accomplish is with this example (compiled with Visual Studio 2008 SP1):

struct ELEMENT1{
    //Its members

    ELEMENT1()
    {
        //Constructor code
    }
    ~ELEMENT1()
    {
        //Destructor code
    }
};


std::map<std::wstring, ELEMENT1> map;

std::pair<std::map<std::wstring, ELEMENT1>::iterator, bool> resIns;
ELEMENT1 element;
std::wstring strKey;

for(size_t i = 0; i < numberRepetitions; i++)
{
    //Do processing
    //...

    //set 'strKey'

    //Insert new element into the map first
    resIns = map.insert(std::pair<std::wstring, ELEMENT1>(strKey, element));    //This line calls ELEMENT1 constructor & destructor twice

    //Then fill out the data
    fill_in_data(resIns.first->second);
}


BOOL fill_in_data(ELEMENT1& outInfo)
{
    //Fill in 'outInfo' -- MUST be in its own function
    //...
}

My goal is to optimize this code, and thus I did the following:

But when I compile this for a Release build and check the assembler code, I can see that the C++ line with map.insert() function calls ELEMENT1 constructor twice! and then twice its destructor. So the following machine code is just for that map.insert() line:

enter image description here

So I'm obviously not seeing something here.

Can someone suggest what's going on in that compiled code & if it's possible to optimize it?

Upvotes: 2

Views: 771

Answers (3)

zett42
zett42

Reputation: 27766

resIns = map.insert(std::pair<std::wstring, ELEMENT1>(strKey, element));

You are constructing a temporary std::pair whose member second is a ELEMENT1. This causes the copy constructor of ELEMENT1 to be called.

The 2nd call to the copy constructor of ELEMENT1 is when std::map::insert() creates a new element in the map that will be initialized by the temporary std::pair.

You can avoid the duplicate constructor call caused by the temporary by using std::map::operator[] instead:

ELEMENT1& resIns = map[ strKey ];

fill_in_data( resIns );

If strKey doesn't already exist in the map, an ELEMENT1 will be default-constructed directly within the map and a reference to the new object will be returned. The constructor will be called exactly one time.

If strKey already exists in the map, a reference to the existing object will be returned.

Upvotes: 1

NathanOliver
NathanOliver

Reputation: 180630

The reason you have 2 constructor calls is because what you are passing to insert does not match what it need. std::map::insert takes a const value_type& and value_type for a map is 

std::pair<const key_type, element_type>
          ^^^^^ this is important

So, since they do not match you construct one element when you use

std::pair<std::wstring, ELEMENT1>(strKey, element)

and then the compiler calls the copy constructor to convert that into a 

std::pair<const std::wstring, ELEMENT1>

A quick fix is to change the code to

std::pair<const std::wstring, ELEMENT1>(strKey, element)

Which leaves you with one temporary that is constructed and destructed. You can also do as zett42 suggests in their answer to avoid the creation of the temporary entirely.

Upvotes: 2

user7860670
user7860670

Reputation: 37547

You should use emplace to avoid creation on temp objects:

resIns = map.emplace
(
    ::std::piecewise_construct
,   ::std::forward_as_tuple(strKey)
,   ::std::forward_as_tuple()
);

A good reason to switch to newer VS version.

Upvotes: -1

Related Questions