This is an archive of the discontinued LLVM Phabricator instance.

MS ABI: Avoid hashing back reference keys in mangler
ClosedPublic

Authored by K-ballo on Sep 21 2014, 12:36 PM.

Details

Summary

This patch replaces the back reference StringMap from the MS mangler with a SmallVector of strings. My previous patches reduced the number of hashes involved in back reference lookups, this one removes them completely. The back reference map contains at most 10 entries, which are likely to be of varying sizes and different initial subsequences, and which can easily became huge (due to templates and namespaces).

The solution presented is the simplest possible one. Nevertheless, it's enough to reduce compilation times for a particular test case from 11.1s to 9s, versus 8.58s for the Itanium ABI. Possible further improvements include using a sorted vector (carefully to not introduce an extra comparison), storing the string contents in a common arena, and/or keep the string storage in the context for reuse.

Diff Detail

Event Timeline

K-ballo updated this revision to Diff 13912.Sep 21 2014, 12:36 PM
K-ballo retitled this revision from to MS ABI: Avoid hashing back reference keys in mangler.
K-ballo updated this object.
K-ballo edited the test plan for this revision. (Show Details)
K-ballo added reviewers: majnemer, timurrrr, chandlerc.
K-ballo added a subscriber: Unknown Object (MLST).
majnemer added inline comments.Sep 21 2014, 12:49 PM
lib/AST/MicrosoftMangle.cpp
192–194

It's not really a map anymore, perhaps BackRefVec ?

timurrrr edited edge metadata.Sep 22 2014, 3:22 AM

Funny.

Agreed to David's comment, otherwise LGTM.

lib/AST/MicrosoftMangle.cpp
999

nit: {} not needed anymore?

K-ballo updated this revision to Diff 13943.Sep 22 2014, 11:10 AM
K-ballo edited edge metadata.

Addressed review comments

majnemer accepted this revision.Sep 22 2014, 1:50 PM
majnemer edited edge metadata.

LGTM

This revision is now accepted and ready to land.Sep 22 2014, 1:50 PM