r233228 - Keep track of canonical decls in Redeclarable.
Sean Silva
chisophugis at gmail.com
Wed Mar 25 18:47:49 PDT 2015
On Wed, Mar 25, 2015 at 6:10 PM, Richard Smith <richard at metafoo.co.uk>
wrote:
> On Wed, Mar 25, 2015 at 6:01 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> On Wed, Mar 25, 2015 at 5:56 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>> Awesome performance numbers!
>>>
>>> When we discussed this previously, one alternative (should we ever need
>>> it; not sure we do) if the memory overhead is excessive is to use a
>>> waymarking scheme to find the canonical decl in constant time (unlike Use
>>> http://www.llvm.org/docs/ProgrammersManual.html#the-waymarking-algorithm
>>> , we don't have random access, but we can instead take advantage of the
>>> stable address of the canonical decl and directly store bits of the stable
>>> address).
>>>
>>
>> D'oh, just saw the review thread and
>> https://llvm.org/bugs/show_bug.cgi?id=21264 where Richard suggests a
>> more elaborate (but not guaranteed constant time?) scheme.
>>
>
> It's guaranteed constant time to get the canonical decl (because every
> decl either is it or has a pointer to it) and to find the previous decl
> (although the constant on that is a bit higher if there are multiple
> redeclarations, as is the memory usage).
>
Doesn't your suggested scheme require a hash table lookup for
getPreviousDecl? That isn't guaranteed constant time.
I was thinking of something cruder like having the low two bits of the
pointers in the chain be {0,bit from address} or {1,bit from address}, with
{1,bit from address} meaning "if you have already collected 32/64 bits of
the address, go ahead and reinterpret them as a pointer to the canonical
decl". That has constant time finding of the canonical decl, constant time
previous decl, and doesn't require any auxiliary data structures on the
side. Although I'm not sure how practical of a "constant time" 10's of
linked-list follows are (and not to mention maintaining that structure,
which takes a similar "constant time").
-- Sean Silva
>
>
>> -- Sean Silva
>>
>>
>>>
>>> -- Sean Silva
>>>
>>> On Wed, Mar 25, 2015 at 4:18 PM, Manuel Klimek <klimek at google.com>
>>> wrote:
>>>
>>>> Author: klimek
>>>> Date: Wed Mar 25 18:18:30 2015
>>>> New Revision: 233228
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=233228&view=rev
>>>> Log:
>>>> Keep track of canonical decls in Redeclarable.
>>>>
>>>> More than 2x speedup on modules builds with large redecl chains.
>>>> Roughly 15-20% speedup on non-modules builds for very large TUs.
>>>> Between 2-3% cost in memory on large TUs.
>>>>
>>>> Modified:
>>>> cfe/trunk/include/clang/AST/Decl.h
>>>> cfe/trunk/include/clang/AST/Redeclarable.h
>>>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>>>
>>>> Modified: cfe/trunk/include/clang/AST/Decl.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=233228&r1=233227&r2=233228&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>>>> +++ cfe/trunk/include/clang/AST/Decl.h Wed Mar 25 18:18:30 2015
>>>> @@ -3738,8 +3738,6 @@ void Redeclarable<decl_type>::setPreviou
>>>> assert(RedeclLink.NextIsLatest() &&
>>>> "setPreviousDecl on a decl already in a redeclaration chain");
>>>>
>>>> - decl_type *First;
>>>> -
>>>> if (PrevDecl) {
>>>> // Point to previous. Make sure that this is actually the most
>>>> recent
>>>> // redeclaration, or we can build invalid chains. If the most
>>>> recent
>>>>
>>>> Modified: cfe/trunk/include/clang/AST/Redeclarable.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Redeclarable.h?rev=233228&r1=233227&r2=233228&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/AST/Redeclarable.h (original)
>>>> +++ cfe/trunk/include/clang/AST/Redeclarable.h Wed Mar 25 18:18:30 2015
>>>> @@ -121,14 +121,15 @@ protected:
>>>> ///
>>>> /// If there is only one declaration, it is <pointer to self, true>
>>>> DeclLink RedeclLink;
>>>> + decl_type *First;
>>>>
>>>> decl_type *getNextRedeclaration() const {
>>>> return RedeclLink.getNext(static_cast<const decl_type *>(this));
>>>> }
>>>>
>>>> public:
>>>> - Redeclarable(const ASTContext &Ctx)
>>>> - : RedeclLink(LatestDeclLink(Ctx)) {}
>>>> + Redeclarable(const ASTContext &Ctx)
>>>> + : RedeclLink(LatestDeclLink(Ctx)), First(static_cast<decl_type
>>>> *>(this)) {}
>>>>
>>>> /// \brief Return the previous declaration of this declaration or
>>>> NULL if this
>>>> /// is the first declaration.
>>>> @@ -144,21 +145,11 @@ public:
>>>>
>>>> /// \brief Return the first declaration of this declaration or
>>>> itself if this
>>>> /// is the only declaration.
>>>> - decl_type *getFirstDecl() {
>>>> - decl_type *D = static_cast<decl_type*>(this);
>>>> - while (D->getPreviousDecl())
>>>> - D = D->getPreviousDecl();
>>>> - return D;
>>>> - }
>>>> + decl_type *getFirstDecl() { return First; }
>>>>
>>>> /// \brief Return the first declaration of this declaration or
>>>> itself if this
>>>> /// is the only declaration.
>>>> - const decl_type *getFirstDecl() const {
>>>> - const decl_type *D = static_cast<const decl_type*>(this);
>>>> - while (D->getPreviousDecl())
>>>> - D = D->getPreviousDecl();
>>>> - return D;
>>>> - }
>>>> + const decl_type *getFirstDecl() const { return First; }
>>>>
>>>> /// \brief True if this is the first declaration in its
>>>> redeclaration chain.
>>>> bool isFirstDecl() const { return RedeclLink.NextIsLatest(); }
>>>>
>>>> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=233228&r1=233227&r2=233228&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
>>>> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Mar 25 18:18:30
>>>> 2015
>>>> @@ -2106,6 +2106,7 @@ ASTDeclReader::VisitRedeclarable(Redecla
>>>> // which is the one that matters and mark the real previous DeclID
>>>> to be
>>>> // loaded & attached later on.
>>>> D->RedeclLink = Redeclarable<T>::PreviousDeclLink(FirstDecl);
>>>> + D->First = FirstDecl->getCanonicalDecl();
>>>> }
>>>>
>>>> // Note that this declaration has been deserialized.
>>>> @@ -2209,6 +2210,7 @@ void ASTDeclReader::mergeRedeclarable(Re
>>>> // of the existing declaration, so that this declaration has the
>>>> // appropriate canonical declaration.
>>>> D->RedeclLink = Redeclarable<T>::PreviousDeclLink(ExistingCanon);
>>>> + D->First = ExistingCanon;
>>>>
>>>> // When we merge a namespace, update its pointer to the first
>>>> namespace.
>>>> // We cannot have loaded any redeclarations of this declaration
>>>> yet, so
>>>> @@ -2828,6 +2830,7 @@ void ASTDeclReader::attachPreviousDeclIm
>>>> Redeclarable<DeclT> *D,
>>>> Decl *Previous, Decl
>>>> *Canon) {
>>>> D->RedeclLink.setPrevious(cast<DeclT>(Previous));
>>>> + D->First = cast<DeclT>(Previous)->First;
>>>> }
>>>> namespace clang {
>>>> template<>
>>>> @@ -2838,6 +2841,7 @@ void ASTDeclReader::attachPreviousDeclIm
>>>> FunctionDecl *PrevFD = cast<FunctionDecl>(Previous);
>>>>
>>>> FD->RedeclLink.setPrevious(PrevFD);
>>>> + FD->First = PrevFD->First;
>>>>
>>>> // If the previous declaration is an inline function declaration,
>>>> then this
>>>> // declaration is too.
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150325/a1f684a4/attachment.html>
More information about the cfe-commits
mailing list