[cfe-commits] r54794 - in /cfe/trunk: include/clang/Lex/Token.h include/clang/Parse/Parser.h lib/Lex/Lexer.cpp lib/Parse/Parser.cpp lib/Sema/Sema.cpp lib/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp

[Daniel Dunbar]

> Maybe these should be moved into a new object owned by ASTContext. 
> The nice thing about doing this is that we could actually fix some 
> efficiency issues as well.  At startup time, various subsystems (e.g. 
> Sema) look up a bunch of interned strings for future comparisons. 
> However, if they are never used, this lookup is wasted, and it could 
> cause an extra string to be interned.  When clang is used as a 
> library, this can be a significant startup time hit.

I agree that they should probably move somewhere else, but I'm not
sure the overhead of the interned strings is so much as to be worth
complicating the code much. And lazily initializing them adds code 
overhead of its own. 

If this was an issue we could also modify IdentifierTable  to quickly intern 
some set of strings on startup, don't you think? Since many applications
will likely end up interning most of the strings this would probably be an
overall win for startup time (although again, I think this is a very minor
factor).

 - Daniel

----- Original Message ----
From: Chris Lattner <clattner at apple.com>
To: Daniel Dunbar <daniel at zuster.org>
Cc: cfe-commits at cs.uiuc.edu
Sent: Saturday, August 23, 2008 5:00:31 PM
Subject: Re: [cfe-commits] r54794 - in /cfe/trunk: include/clang/Lex/Token.h include/clang/Parse/Parser.h lib/Lex/Lexer.cpp lib/Parse/Parser.cpp lib/Sema/Sema.cpp lib/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp


On Aug 14, 2008, at 3:04 PM, Daniel Dunbar wrote:

> Author: ddunbar
> Date: Thu Aug 14 17:04:54 2008
> New Revision: 54794
>
> URL: http://llvm.org/viewvc/llvm-project?rev=54794&view=rev
> Log:
> Change Parser & Sema to use interned "super" for comparions.
> - Added as private members for each because it is not clear where to
>  put the common definition. Perhaps the IdentifierInfos all of these
>  "pseudo-keywords" should be collected into one place (this would
>  KnownFunctionIDs and Objective-C property IDs, for example)

Nice change!

Maybe these should be moved into a new object owned by ASTContext. 
The nice thing about doing this is that we could actually fix some 
efficiency issues as well.  At startup time, various subsystems (e.g. 
Sema) look up a bunch of interned strings for future comparisons. 
However, if they are never used, this lookup is wasted, and it could 
cause an extra string to be interned.  When clang is used as a 
library, this can be a significant startup time hit.

I admit that this is mostly an issue for the preprocessor, and is 
basically unavoidable there (it had to add all the predefined macros 
to the identifier table and macro tables), but it would be nice to not 
abuse IdentifierTable any more than we have to.

-Chris