Reviewer's choice:
http://reviews.llvm.org/D16360
(I've barely used phab, so let me know if it's broken somehow...)
> On 2016-Jan-19, at 16:15, Eric Fiselier <e...@efcs.ca> wrote:
>
> Hey Duncan,
>
> I know this isn't required, but would it be possible to put this on
> phabricator?
>
> /Eric
>
> On Mon, Jan 18, 2016 at 4:34 PM, Duncan P. N. Exon Smith
> <dexonsm...@apple.com> wrote:
> ping
>
> > On 2016-Jan-11, at 16:23, Duncan P. N. Exon Smith <dexonsm...@apple.com>
> > wrote:
> >
> > ping
> >
> >> On 2016-Jan-04, at 12:37, Duncan P. N. Exon Smith <dexonsm...@apple.com>
> >> wrote:
> >>
> >> ping
> >>
> >>> On 2015-Dec-17, at 13:56, Duncan P. N. Exon Smith <dexonsm...@apple.com>
> >>> wrote:
> >>>
> >>>
> >>>> On 2015-Dec-16, at 14:42, Duncan P. N. Exon Smith <dexonsm...@apple.com>
> >>>> wrote:
> >>>>
> >>>> This is a follow-up to r239666: "Fix PR12999 - unordered_set::insert
> >>>> calls operator new when no insert occurs". That fix didn't apply to
> >>>> `unordered_map` because unordered_map::value_type gets packed inside:
> >>>> --
> >>>> union __value_type {
> >>>> pair<key_type, mapped_type> __nc; // Only C++11 or higher.
> >>>> pair<const key_type, mapped_type> __cc; // Always.
> >>>> // Constructors...
> >>>> };
> >>>> --
> >>>> and the underlying __hash_table only knows about __value_type.
> >>>
> >>> Sorry for the quick ping, but I realized this morning that my approach
> >>> was still leaving mallocs on the table.
> >>>
> >>> I've attached a new patch that handles more cases.
> >>>
> >>> This patch should avoid unnecessary mallocs whenever the caller passes
> >>> in a pair<T, U> such that T is trivially convertible to key_type.
> >>>
> >>> Since __hash_table's value_type is really *never* a pair (for
> >>> unordered_map, it's a union of two pairs) the static dispatch is all in
> >>> unordered_map. It's doing this:
> >>> - If the argument isn't a pair<>, alloc.
> >>> - If argument.first can be referenced as const key_type&, don't alloc.
> >>> - If argument.first can be trivially converted to key_type, don't
> >>> alloc.
> >>> - Else alloc.
> >>>
> >>> In the pre-C++11 world the caller has already converted to
> >>> unordered_map::value_type. We can always avoid the alloc.
> >>>
> >>> To support all of this:
> >>> - In C++03, __unordered_map_equal and __unordered_map_hasher need to
> >>> handle unordered_map::value_type.
> >>> - In C++03, __hash_table::__insert_unique_value() now takes its
> >>> argument by template.
> >>> - In C++11, __hash_table::__insert_unique_value() is now a one-liner
> >>> that forwards to __insert_unique_key_value() for the real work.
> >>> - The versions of __hash_table::__construct_node() that take a
> >>> pre-computed hash have been renamed to __construct_node_hash(), and
> >>> these versions use perfect forwarding.
> >>>
> >>> Most of the following still apply:
> >>>
> >>>> This is one of my first patches for libc++, and I'm not sure of a few
> >>>> things:
> >>>> - Did I successfully match the coding style? (I'm kind of lost
> >>>> without clang-format TBH.)
> >>>> - Should I separate the change to __construct_node_hash() into a
> >>>> separate prep commit? (I would if this were LLVM, but I'm not sure
> >>>> if the common practice is different for libc++.)
> >>>> - Most of the overloads I added to __unordered_map_hasher and
> >>>> __unordered_map_equal aren't actually used by
> >>>> __hash_table::__insert_unique_value(). Should I omit the unused
> >>>> ones? (Again, for LLVM I would have omitted them.)
> >>>
> >>> (For the updated patch, I went with the LLVM approach of only adding
> >>> the used API. It seems more appropriate in this case.)
> >>>
> >>>> After this I'll fix the same performance issue in std::map (and I
> >>>> assume std::set?).
> >>
> >> <0001-unordered_map-Avoid-unnecessary-mallocs-when-no-i-v2.patch>
> >
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
