On 10/2/14 12:42 PM, Eric Fiselier wrote:
>> If this isn't/wasn't going to help out the PMF stuff in std::experimental,
>> then I don't think that this is worth doing.
>
> I agree, I think the best thing to do would be to simply rename the functions
> to avoid name collisions and then implement them separately for the PMR
> stuff. That way we don't have to worry about changing the performance of the
> hash containers.
>
> ================
> Comment at: include/__hash_table:86
> @@ -79,2 +85,3 @@
> {
> - return size_t(1) << (std::numeric_limits<size_t>::digits - __clz(__n-1));
> + return __n < 2 ? __n + (__n == 0)
> + : size_t(1) << (std::numeric_limits<size_t>::digits - __clz(__n-1));
> ----------------
> mclow.lists wrote:
>> Looks to me that this produces:
>> 0 --> 1
>> 1 --> 1
>> 2 --> 4
>> 4 --> 8
>>
>> Is that the intended behavior? (especially the second one)
Howard mentioned something about this earlier in this thread:
>Just a word of caution:
>
>These functions were not intended to be mathematically accurate. They were
>intended to make the unordered containers as fast as possible. Performance
>tests (not checked in) were used to tune these functions. Put one too many
>branches in a hot path, and you can easily destroy the performance gains that
>were intended by allowing power-of-2 bucket sizes. I’m not saying this patch
>does this. I’m just saying, you are in danger of doing so if you haven’t
>performance tested these changes.
>
>Also, for the purpose of the libc++ unordered containers:
>
>0 is a valid bucket count. An upward bump of 0 buckets goes to 2.
>1 is considered not a power of 2, but it rounded up to 2.
>2 is considered prime, not a power of 2. Thus a upward bump off bucket size 2
>goes to 5, not 4.
>
>For all other bucket sizes, it is unambiguous as to whether one is dealing
>with a prime or power of 2.
>
>These definitions are intended to promote best practices for the libc++
>unordered containers, as opposed to being mathematically accurate.
>
>Howard
> Oops, I don't think so.
> ```
> return n < 2 ? __n + 1
> ```
> Should fix that.
>
> But IDK if the change is needed. However I am still concerned that the
> current version exhibits undefined behaviour for `__n = 1`.
>
> http://reviews.llvm.org/D4948
>
>
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
--
Jon Roelofs
[email protected]
CodeSourcery / Mentor Embedded
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
