On Apr 17, 2013, at 4:07 PM, David Blaikie <[email protected]> wrote:
> On Wed, Apr 17, 2013 at 3:10 PM, Argyrios Kyrtzidis <[email protected]> wrote: >> Author: akirtzidis >> Date: Wed Apr 17 17:10:55 2013 >> New Revision: 179730 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=179730&view=rev >> Log: >> [Modules] Use global index to improve typo correction performance >> >> Typo correction for an unqualified name needs to walk through all of the >> identifier tables of all modules. >> When we have a global index, just walk its identifier table only. >> >> rdar://13425732 >> >> Modified: >> cfe/trunk/include/clang/Basic/IdentifierTable.h >> cfe/trunk/include/clang/Serialization/ASTReader.h >> cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h >> cfe/trunk/lib/Basic/IdentifierTable.cpp >> cfe/trunk/lib/Serialization/ASTReader.cpp >> cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp >> >> Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=179730&r1=179729&r2=179730&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/IdentifierTable.h (original) >> +++ cfe/trunk/include/clang/Basic/IdentifierTable.h Wed Apr 17 17:10:55 2013 >> @@ -394,7 +394,7 @@ public: >> /// >> /// \returns A new iterator into the set of known identifiers. The >> /// caller is responsible for deleting this iterator. >> - virtual IdentifierIterator *getIdentifiers() const; >> + virtual IdentifierIterator *getIdentifiers(); >> }; >> >> /// \brief An abstract class used to resolve numerical identifier >> >> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=179730&r1=179729&r2=179730&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original) >> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Apr 17 17:10:55 >> 2013 >> @@ -1558,7 +1558,7 @@ public: >> >> /// \brief Retrieve an iterator into the set of all identifiers >> /// in all loaded AST files. >> - virtual IdentifierIterator *getIdentifiers() const; >> + virtual IdentifierIterator *getIdentifiers(); >> >> /// \brief Load the contents of the global method pool for a given >> /// selector. >> >> Modified: cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h?rev=179730&r1=179729&r2=179730&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h (original) >> +++ cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h Wed Apr 17 >> 17:10:55 2013 >> @@ -146,6 +146,11 @@ public: >> static std::pair<GlobalModuleIndex *, ErrorCode> >> readIndex(StringRef Path); >> >> + /// \brief Returns an iterator for identifiers stored in the index table. >> + /// >> + /// The caller accepts ownership of the returned object. >> + IdentifierIterator *createIdentifierIterator() const; >> + >> /// \brief Retrieve the set of modules that have up-to-date indexes. >> /// >> /// \param ModuleFiles Will be populated with the set of module files that >> >> Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=179730&r1=179729&r2=179730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Basic/IdentifierTable.cpp (original) >> +++ cfe/trunk/lib/Basic/IdentifierTable.cpp Wed Apr 17 17:10:55 2013 >> @@ -65,7 +65,7 @@ namespace { >> }; >> } >> >> -IdentifierIterator *IdentifierInfoLookup::getIdentifiers() const { >> +IdentifierIterator *IdentifierInfoLookup::getIdentifiers() { >> return new EmptyLookupIterator(); >> } >> >> >> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=179730&r1=179729&r2=179730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Apr 17 17:10:55 2013 >> @@ -6109,7 +6109,10 @@ StringRef ASTIdentifierIterator::Next() >> return Result; >> } >> >> -IdentifierIterator *ASTReader::getIdentifiers() const { >> +IdentifierIterator *ASTReader::getIdentifiers() { >> + if (!loadGlobalIndex()) >> + return GlobalIndex->createIdentifierIterator(); > > This sort of feels like it still ought to be conceptually const, > perhaps? (ie: loading the global index (I haven't actually looked at > the implementation details of that function) is an implementation > detail that doesn't change the "state" of the ASTReader, does it? > Well, I suppose it's a 'reader' so its constness is non-obvious to > begin with). Just a thought. This is implementing the method from the abstract class IdentifierInfoLookup (ASTReader is an implementation of it). Not a strong opinion but I'm leaning towards that IdentifierInfoLookup should not dictate to its implementations that the "getIdentifiers()" method implementation ought to be const. It did not for the "get()" method. > >> + >> return new ASTIdentifierIterator(*this); >> } >> >> >> Modified: cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp?rev=179730&r1=179729&r2=179730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp (original) >> +++ cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp Wed Apr 17 17:10:55 >> 2013 >> @@ -818,3 +818,34 @@ GlobalModuleIndex::writeIndex(FileManage >> // We're done. >> return EC_None; >> } >> + >> +namespace { >> + class GlobalIndexIdentifierIterator : public IdentifierIterator { >> + /// \brief The current position within the identifier lookup table. >> + IdentifierIndexTable::key_iterator Current; >> + >> + /// \brief The end position within the identifier lookup table. >> + IdentifierIndexTable::key_iterator End; >> + >> + public: >> + explicit GlobalIndexIdentifierIterator(IdentifierIndexTable &Idx) { >> + Current = Idx.key_begin(); >> + End = Idx.key_end(); >> + } >> + >> + virtual StringRef Next() { >> + if (Current == End) >> + return StringRef(); >> + >> + StringRef Result = *Current; >> + ++Current; >> + return Result; >> + } >> + }; >> +} >> + >> +IdentifierIterator *GlobalModuleIndex::createIdentifierIterator() const { >> + IdentifierIndexTable &Table = >> + *static_cast<IdentifierIndexTable *>(IdentifierIndex); >> + return new GlobalIndexIdentifierIterator(Table); >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
