Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove useless allocator copy in map #49

Merged

Conversation

joker-eph
Copy link
Contributor

I never contributed to boost, I'm not sure what kind of test I should write for this? For now, I ran successfully b2 in boost/libs/container/test with this patch applied.

This patch adds a variant of map constructors to avoid useless extra allocator copy when using initializer list

Many existing constructors have this form:

map(std::initializer_list<value_type> il, const Compare& comp = Compare(), const allocator_type& a = allocator_type())

The issue is that a temporary allocator_type is constructed, and
passed to the base class where it is used to copy-constructed the
rebound allocator.
This temporary allocator_type here is always destroyed at the end
of the map() constructor. For stateful allocators this is not
desirable.
The solution is to adopt what libc++ is doing and have to constructors:

map(std::initializer_list<value_type> il, const Compare& comp = Compare(), const allocator_type& a)

and

map(std::initializer_list<value_type> il, const Compare& comp = Compare())

This way, unless an allocator is provided by the client, no extra temporary
creation/destruction occurs.

@joker-eph joker-eph force-pushed the remove_useless_allocator_copy_in_map branch 2 times, most recently from 4404c69 to 59020dd Compare May 8, 2017 19:36
… when using initializer list

Many existing constructors have this form:

  map(std::initializer_list<value_type> il, const Compare& comp = Compare(), const allocator_type& a = allocator_type())

The issue is that a temporary allocator_type is constructed, and
passed to the base class where it is used to copy-constructed the
rebound allocator.
This temporary allocator_type here is always destroyed at the end
of the map() constructor. For stateful allocators this is not
desirable.
The solution is to adopt what libc++ is doing and have to constructors:

  map(std::initializer_list<value_type> il, const Compare& comp = Compare(), const allocator_type& a)

and

  map(std::initializer_list<value_type> il, const Compare& comp = Compare())

This way, unless an allocator is provided by the client, no extra temporary
creation/destruction occurs.
@joker-eph joker-eph force-pushed the remove_useless_allocator_copy_in_map branch from 59020dd to 40451e4 Compare May 8, 2017 19:39
@igaztanaga igaztanaga merged commit 40451e4 into boostorg:develop May 16, 2017
@igaztanaga
Copy link
Member

Thanks for the patch. I applied it and added some additional changes after reviewing all associative (tree-based and flat_tree based) containers.

@joker-eph joker-eph deleted the remove_useless_allocator_copy_in_map branch June 2, 2017 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants