[PATCH] D16571: Compute the DISubprogram hash key partially (NFC)

[Daniel Berlin via llvm-commits]

On Tue, Jan 26, 2016 at 3:35 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

> The hash should (almost?) never change.
> I thought about this solution, and I plan on evaluating it a well (it
> applies to constants as well).
> However there is a tradeoff of memory vs speed doing this.
> For instance when doing LTO, we load a lot of metadata when loading and
> linking individual Modules, which put a lot of stress on the hash tables
> (both uniquing and grow). But then during optimization and CodeGen it
> shouldn’t and I’m not sure we want to pay the price of the memory overhead
> then.
>
> This naturally raises the questions of whether you really need hash tables
to get what you want, or would some other more memory-efficient data
structure be better for uniquing :)

While DenseMap is pretty good,  you do end up with a lot of empty buckets
(or pay a high probing price).

Whereas, depending on the situation and structure, you can often pay a much
lower cost (and have better behavior on hash misses). Obviously, in most
cases, DenseMap should be the choice because it's consistent and who cares,
but in specific situations if you are placing huge stress on it during say,
uniquing, there are often better ways both in time and space (various
compressed tries, ternary search trees, etc).


Note also that a ThinLTO link of `opt` right now involves loading ~30000
> bitcode files, so it is roughly 30s CPU time overall for the link time for
> each millisecond saved for each file loaded :)
>
> Let me know if you have any other suggestion!
>
> —
> Mehdi
>
>
> On Jan 26, 2016, at 1:57 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>
> How often does the hash change?
> If it's really speed important,  you could:
> A. store the hash value
> B. update it when fields change.
>
>
> On Tue, Jan 26, 2016 at 1:23 AM, Mehdi AMINI via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> joker.eph created this revision.
>> joker.eph added a reviewer: dexonsmith.
>> joker.eph added a subscriber: llvm-commits.
>>
>> This patch changes the computation of the hash key for DISubprogram to
>> be computed on a small subset of the fields. The hash is computed a
>> lot faster, but there might be more collision in the table.
>> However by carefully selecting the fields, colisions should be rare.
>>
>> Using `opt` to load the IR for FastISelEmitter.cpp.o, this patch
>> improves DISubprogram::getImpl() runtime from 28ms to 15ms.
>>
>> http://reviews.llvm.org/D16571
>>
>> Files:
>>   lib/IR/LLVMContextImpl.h
>>
>> Index: lib/IR/LLVMContextImpl.h
>> ===================================================================
>> --- lib/IR/LLVMContextImpl.h
>> +++ lib/IR/LLVMContextImpl.h
>> @@ -519,10 +519,7 @@
>>             Variables == RHS->getRawVariables();
>>    }
>>    unsigned getHashValue() const {
>> -    return hash_combine(Scope, Name, LinkageName, File, Line, Type,
>> -                        IsLocalToUnit, IsDefinition, ScopeLine,
>> ContainingType,
>> -                        Virtuality, VirtualIndex, Flags, IsOptimized,
>> -                        TemplateParams, Declaration, Variables);
>> +    return hash_combine(Scope, File, Type, Line);
>>    }
>>  };
>>
>>
>>
>>
>> _______________________________________________
>> 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/20160126/e216ba0c/attachment.html>