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

Reply via email to