r179730 - [Modules] Use global index to improve typo correction performance

[David Blaikie]

On Wed, Apr 17, 2013 at 3:10 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> 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.

> +
>    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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits