[llvm] r310189 - [ADT] Add a much simpler loop to DenseMap::clear when the types are

[Chandler Carruth via llvm-commits]

On Mon, Aug 7, 2017 at 11:33 AM David Blaikie via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On Sat, Aug 5, 2017 at 3:49 PM Chandler Carruth via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: chandlerc
>> Date: Sat Aug  5 15:48:37 2017
>> New Revision: 310189
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=310189&view=rev
>> Log:
>> [ADT] Add a much simpler loop to DenseMap::clear when the types are
>> POD-like and we can just splat the empty key across memory.
>>
>> Sadly we can't optimize the normal loop well enough because we can't
>> turn the conditional store into an unconditional store according to the
>> memory model.
>>
>> This loop actually showed up in a profile of code that was calling clear
>> as a serious source of time. =[
>>
>> Modified:
>>     llvm/trunk/include/llvm/ADT/DenseMap.h
>>
>> Modified: llvm/trunk/include/llvm/ADT/DenseMap.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=310189&r1=310188&r2=310189&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/DenseMap.h (original)
>> +++ llvm/trunk/include/llvm/ADT/DenseMap.h Sat Aug  5 15:48:37 2017
>> @@ -107,17 +107,23 @@ public:
>>      }
>>
>>      const KeyT EmptyKey = getEmptyKey(), TombstoneKey =
>> getTombstoneKey();
>> -    unsigned NumEntries = getNumEntries();
>> -    for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P) {
>> -      if (!KeyInfoT::isEqual(P->getFirst(), EmptyKey)) {
>> -        if (!KeyInfoT::isEqual(P->getFirst(), TombstoneKey)) {
>> -          P->getSecond().~ValueT();
>> -          --NumEntries;
>> -        }
>> +    if (isPodLike<KeyT>::value && isPodLike<ValueT>::value) {
>>
>
> What if these conditionals ^ were rolled into the general form of the loop
> in the else block? So that the dtor wouldn't be invoked for pod value
> types, even if the key wasn't pod - and the key would be assigned to
> EmptyKey without an isEqual check if the key is pod even if the value isn't.
>

I was worried about assigning over that many keys unnecessarily if that
assignment might be slow (IE, call an assignment operator). The POD-ness is
admittedly a poor proxy for this... other ideas?


>
>
>> +      // Use a simpler loop when these are trivial types.
>> +      for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P)
>>          P->getFirst() = EmptyKey;
>> +    } else {
>> +      unsigned NumEntries = getNumEntries();
>> +      for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P)
>> {
>> +        if (!KeyInfoT::isEqual(P->getFirst(), EmptyKey)) {
>> +          if (!KeyInfoT::isEqual(P->getFirst(), TombstoneKey)) {
>> +            P->getSecond().~ValueT();
>> +            --NumEntries;
>> +          }
>> +          P->getFirst() = EmptyKey;
>> +        }
>>        }
>> +      assert(NumEntries == 0 && "Node count imbalance!");
>>      }
>> -    assert(NumEntries == 0 && "Node count imbalance!");
>>      setNumEntries(0);
>>      setNumTombstones(0);
>>    }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170807/4feac756/attachment.html>