eric_niebler added inline comments. ================ Comment at: lib/Lex/PPDirectives.cpp:33 @@ -28,2 +32,2 @@ #include "llvm/Support/Path.h" #include "llvm/Support/SaveAndRestore.h" ---------------- rsmith wrote: > eric_niebler wrote: > > You mean, instead of the `StringSet` below? Looks like `StringSwitch` just > > turns into cascading `memcmp`'s. Bet I can tell you how that performs > > versus a hash set. :-) Also, with the `StringSet`, I get to initialize it > > once and reuse it many times. I expect that will be pretty darn quick at > > runtime, but I'm looking forward to @bruno's results. > Right, I'm not suggesting `StringSwitch` will be faster; it's preferable for > other reasons (it avoids the memory and shutdown costs of the static local > set). We should stick with what you have if the performance advantage is > measurable; otherwise my preference would be to use `StringSwitch`. But it's > only a slight preference -- if you'd rather not, I won't complain. > > [`StringSwitch` isn't /quite/ as bad as you're suggesting: it always first > compares on length, and it typically compiles into a switch on string length > followed by memcmps. Moreover, the code should be "obvious" enough that > compilers can (at least in principle) optimize those memcmps very > aggressively, right down into the equivalent of an unrolled DFA or a perfect > hash function, but I'm not at all confident that LLVM will actually do that > =)] @rsmith wrote: > We should stick with what you have if the performance advantage is > measurable; otherwise my preference would be to use `StringSwitch`.
I tried `StringSwitch` on Windows.h where there are lots of wrongly-cased #includes. I couldn't measure the difference. I'll update the diff. http://reviews.llvm.org/D19843 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits