Skip to content

map operator[] (both non-templated versions) don't provide a proper hint to the rbtree DoInsertKey(true_type, const_iterator position, const key_type& key) #592

@bochsler

Description

@bochsler

both version of map operator[] (map.h line 184 and 185) use lower_bound(key) to detect if a lower bound already exists and then attempt to use it as a hint for faster insertion, both call down into the rbtree base_type::DoInsertKey(true_type(), itLower, key) or the same thing but eastl::move(key) for the rvalue version. DoInsertKey(...) then calls DoGetKeyInsertionPositionUniqueKeysHint(...) in order to potentially (hopefully) use the provided iterator hint to quickly find the parent node for very fast insertion. The issue is that DoGetKeyInsertionPositionUniqueKeysHint(...) expects the hint to be immediately less than the incoming key, and lower_bound provides an iterator that is >= the key.

by modifying line 559 of map.h in master branch to:

if (itLower != begin())
	itLower = base_type::DoInsertKey(true_type(), --itLower, eastl::move(key));
else
	itLower = base_type::DoInsertKey(true_type(), itLower, eastl::move(key));

I can successfully trigger line 1534 in red_black_tree.h inside DoGetKeyInsertionPositionUniqueKeysHint(...), by inserting on key 7 in belows example, to provide a parent hint in order to prevent another log base 2 search from needing to take place. Perhaps this would be a fix for this? Also maybe if itLower == begin() attempting a hint would just be a waste of time?
Here is the test code I used:

eastl::map<int, char> map1;

for (uint32_t i = 0; i < 15; i+=2)
{
	map1.insert(eastl::make_pair(i, 'A' + i));
}

map1[7] = 'Z';

Thanks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions