[cfe-commits] r61929 - in /cfe/trunk: Driver/RewriteObjC.cpp include/clang/AST/DeclBase.h include/clang/AST/DeclObjC.h lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclObjC.cpp lib/CodeGen/CGObjCGNU.cpp lib/Sema/IdentifierResolver.cpp lib/Sem

[Douglas Gregor]

Hi Steve, Daniel,

On Jan 8, 2009, at 6:04 PM, steve naroff wrote:

>
> On Jan 8, 2009, at 8:20 PM, Daniel Dunbar wrote:
>
>> Hi Steve,
>>
>> My tester is pointing at this for a .98% slowdown at -fsyntax-only  
>> (of
>> parts of Sketch, essentially Cocoa.h). I was really hoping we were
>> going to see a speedup from this! Can you investigate?
>>
> I didn't expect a speedup. From my perspective, this change was done
> for "software engineering" reasons (easier to work with, maintain,
> more consistent with the core lookup mechanism).
>
> ObjCInterfaceDecl::lookupInstanceMethod() and
> ObjCContainerDecl::getInstanceMethod() are used on a per-message basis
> (when the object is statically typed). Even though this code path
> should be faster (no linear search), it is dwarfed by the overhead of
> processing "Cocoa.h".
>
> The only change that *is* proportional to the number of methods is the
> following: we now use DeclContext::addDecl() instead of insMethods
> (which was an llvm::SmallVector)...

DeclContext::addDecl currently pushes into an std::vector and then, if  
we've done lookups into the DeclContext, also pushes into a DenseMap.  
The first order of business is to see if we're actually doing those  
DenseMap lookups.

>
>>> -        insMethods.push_back(Method);
>>> +        DC->addDecl(Context, Method);
>
> In addition, all ObjCMethodDecls are now ScopedDecls.
>
> Doug, has DeclContext::insertImpl() been tuned?

No. We're still using an std::vector for storing declarations (to be  
replaced with a linked list in the future) and, if we're performing  
lookups into the DeclContext, we're also paying for the heavyweight   
the DenseMap<DeclarationName, std::vector<StoredDecl*>>.

>
> I believe the slowdown must be related to changes surrounding
> ObjCMethodDecl...


It's certainly possible. I'll take a look at this tomorrow.

	- Doug