[llvm-commits] PATCH: Switch StringMap to the new hashing infrastructure

[Chandler Carruth]

On Mon, Mar 5, 2012 at 3:15 AM, Jay Foad <jay.foad at gmail.com> wrote:

> On 5 March 2012 10:17, Chandler Carruth <chandlerc at google.com> wrote:
> > The measured changes in performance are all down into the noise floor,
> > despite everything I did to lower that. From what I can tell, this change
> > has no observable performance impact, and reduces the number of hashing
> > functions in use. Good to commit?
>
> I like it.
>
> I notice that the patch doesn't actually remove HashString, because
> it's still used in loads of other places. Do you plan to convert them
> so that we can *really* "reduce the number of hashing functions in
> use"? (If you don't plan to do it yourself I suppose it would make a
> good beginner's project for someone else.)
>

Yep, I'm steadily working through uses of HashString and DenseMapInfo
converting them as I go. This one is just one of the more sensitive ones.


Also, in the interest of full disclosure, I'm tracking down some
performance issues w/ the current hashing function stemming from poor
inlining decisions when LLVM is optimizing code that calls this... Finally
got it all isolated and can show that its just an inlining issue. Still, it
doesn't impact anything I can measure in an execution of Clang, just
impacts my theoretical micro-benchmarks.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120305/6912d164/attachment.html>