[clangd] Store hash of command line in index shards.

[kadircet]

Summary

This is to enable cache invalidation when command line flags changes.

[sammccall]

This makes sense but is hard to debug - is there a reason we don't just store (the relevant parts of) the actual compile command? Size?

clang-tools-extra/clangd/index/Background.cpp

434

only CommandLine and Directory are relevant.
Heuristic and Output don't affect anything, and Filename is already part of the storage key.

[kadircet]

In D64018#1566085, @sammccall wrote:

This makes sense but is hard to debug - is there a reason we don't just store (the relevant parts of) the actual compile command? Size?

Yes I had the size in mind, also didn't see any use case for the actual compile commands.
As for size, I suppose it is OK to store the whole command as it is only stored for the main file of a TU.

[kadircet]

• Store Cmdline itself instead of hash

[sammccall]

Still LG

clang-tools-extra/clangd/index/Serialization.h

47

Need to document that this contains only Directory and CommandLine (or use another struct)

[sammccall]

clang-tools-extra/clangd/index/Serialization.cpp

416

the packed ArrayRef is a bit weird here.
Can we use a struct containing StringRefs, and drop the irrelevant members of CompileCommand?

[kadircet]

• Use an InternedCompileCommand struct rather than a pack of StringRefs.

[sammccall]

clang-tools-extra/clangd/index/Serialization.cpp

504

nit: push_back, or emplace_back(C)

557

nit: internedcmd to avoid confusion below?

[kadircet]

• Address comments

clang-tools-extra/clangd/index/Background.cpp

 #include "index/IndexAction.h"
 #include "index/MemIndex.h"
 #include "index/Ref.h"
 #include "index/Relation.h"
 #include "index/Serialization.h"
 #include "index/SymbolCollector.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/Threading.h"
for (const auto &FileIt : Files) {
     // FIXME: Also skip serializing the shard if it is already up-to-date.
     if (IndexStorage) {
       IndexFileOut Shard;
       Shard.Symbols = SS.get();
       Shard.Refs = RS.get();
       Shard.Relations = RelS.get();
       Shard.Sources = IG.get();
 
+      // Only store command line hash for main files of the TU, since our
+      // current model keeps only one version of a header file.
+      if (Path == MainFile)
+        Shard.CmdHash = Index.CmdHash.getPointer();
+
       if (auto Error = IndexStorage->storeShard(Path, Shard))
         elog("Failed to write background-index shard for file {0}: {1}", Path,
              std::move(Error));
     }
 
     {
       std::lock_guard<std::mutex> Lock(DigestsMu);
       auto Hash = FileIt.second.Digest;
log("BackgroundIndex: rebuilt symbol index with estimated memory {0} "
         estimateMemoryUsage());
   }
 }
 
 llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd,
                                    BackgroundIndexStorage *IndexStorage) {
   trace::Span Tracer("BackgroundIndex");
   SPAN_ATTACH(Tracer, "file", Cmd.Filename);
+  size_t CmdHash =
+      llvm::hash_combine(Cmd.Filename, llvm::join(Cmd.CommandLine, ""),
+                         Cmd.Directory, Cmd.Heuristic, Cmd.Output);
   auto AbsolutePath = getAbsolutePath(Cmd);
 
   auto FS = FSProvider.getFileSystem();
   auto Buf = FS->getBufferForFile(AbsolutePath);
   if (!Buf)
     return llvm::errorCodeToError(Buf.getError());
   auto Hash = digest(Buf->get()->getBuffer());
 
llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd,
   if (!Action->BeginSourceFile(*Clang, Input))
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "BeginSourceFile() failed");
   if (llvm::Error Err = Action->Execute())
     return Err;
 
   Action->EndSourceFile();
 
+  Index.CmdHash = CmdHash;
   assert(Index.Symbols && Index.Refs && Index.Sources &&
          "Symbols, Refs and Sources must be set.");
 
   log("Indexed {0} ({1} symbols, {2} refs, {3} files)",
       Inputs.CompileCommand.Filename, Index.Symbols->size(),
       Index.Refs->numRefs(), Index.Sources->size());
   SPAN_ATTACH(Tracer, "symbols", int(Index.Symbols->size()));
   SPAN_ATTACH(Tracer, "refs", int(Index.Refs->numRefs()));

clang-tools-extra/clangd/index/Serialization.h

 
 // Holds the contents of an index file that was read.
 struct IndexFileIn {
   llvm::Optional<SymbolSlab> Symbols;
   llvm::Optional<RefSlab> Refs;
   llvm::Optional<RelationSlab> Relations;
   // Keys are URIs of the source files.
   llvm::Optional<IncludeGraph> Sources;
+  llvm::Optional<size_t> CmdHash;
 };
 // Parse an index file. The input must be a RIFF or YAML file.
 llvm::Expected<IndexFileIn> readIndexFile(llvm::StringRef);
 
 // Specifies the contents of an index file to be written.
 struct IndexFileOut {
   const SymbolSlab *Symbols = nullptr;
   const RefSlab *Refs = nullptr;
   const RelationSlab *Relations = nullptr;
   // Keys are URIs of the source files.
   const IncludeGraph *Sources = nullptr;
   // TODO: Support serializing Dex posting lists.
   IndexFileFormat Format = IndexFileFormat::RIFF;
+  const size_t *CmdHash = nullptr;
 
   IndexFileOut() = default;
   IndexFileOut(const IndexFileIn &I)
       : Symbols(I.Symbols ? I.Symbols.getPointer() : nullptr),
         Refs(I.Refs ? I.Refs.getPointer() : nullptr),
         Relations(I.Relations ? I.Relations.getPointer() : nullptr),
-        Sources(I.Sources ? I.Sources.getPointer() : nullptr) {}
+        Sources(I.Sources ? I.Sources.getPointer() : nullptr),
+        CmdHash(I.CmdHash ? I.CmdHash.getPointer() : nullptr) {}
 };
 // Serializes an index file.
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const IndexFileOut &O);
 
 // Convert a single symbol to YAML, a nice debug representation.
 std::string toYAML(const Symbol &);
 std::string toYAML(const std::pair<SymbolID, ArrayRef<Ref>> &);
 std::string toYAML(const Relation &);

clang-tools-extra/clangd/index/Serialization.cpp

 //===-- Serialization.cpp - Binary serialization of index data ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Serialization.h"
 #include "Logger.h"
 #include "RIFF.h"
 #include "SymbolLocation.h"
 #include "SymbolOrigin.h"
 #include "Trace.h"
 #include "dex/Dex.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 llvm::Error makeError(const llvm::Twine &Msg) {

 // A file is a RIFF chunk with type 'CdIx'.
 // It contains the sections:
 //   - meta: version number
 //   - srcs: information related to include graph
 //   - stri: string table
 //   - symb: symbols
 //   - refs: references to symbols
 
 // The current versioning scheme is simple - non-current versions are rejected.
 // If you make a breaking change, bump this version number to invalidate stored
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 10;
+constexpr static uint32_t Version = 11;
 
 llvm::Expected<IndexFileIn> readRIFF(llvm::StringRef Data) {
   auto RIFF = riff::readFile(Data);
   if (!RIFF)
     return RIFF.takeError();
   if (RIFF->Type != riff::fourCC("CdIx"))
     return makeError("wrong RIFF type");
   llvm::StringMap<llvm::StringRef> Chunks;
if (Chunks.count("rela")) {
     while (!RelationsReader.eof()) {
       auto Relation = readRelation(RelationsReader);
       Relations.insert(Relation);
     }
     if (RelationsReader.err())
       return makeError("malformed or truncated relations");
     Result.Relations = std::move(Relations).build();
   }
+  if (Chunks.count("cmdl")) {
+    Reader Cmd(Chunks.lookup("cmdl"));
+    if (Cmd.err())
+      return makeError("malformed or truncated commandline section");
+    Result.CmdHash = Cmd.consume32();
+  }
   return std::move(Result);
 }
 
 template <class Callback>
 void visitStrings(IncludeGraphNode &IGN, const Callback &CB) {
   CB(IGN.URI);
   for (llvm::StringRef &Include : IGN.DirectIncludes)
     CB(Include);
 }
 
 void writeRIFF(const IndexFileOut &Data, llvm::raw_ostream &OS) {
   assert(Data.Symbols && "An index file without symbols makes no sense!");
   riff::File RIFF;
void writeRIFF(const IndexFileOut &Data, llvm::raw_ostream &OS) {
   std::vector<Relation> Relations;
   if (Data.Relations) {
     for (const auto &Relation : *Data.Relations) {
       Relations.emplace_back(Relation);
       // No strings to be interned in relations.
     }
   }
 
   std::string StringSection;
   {
     llvm::raw_string_ostream StringOS(StringSection);
     Strings.finalize(StringOS);
   }
   RIFF.Chunks.push_back({riff::fourCC("stri"), StringSection});
 
   std::string SymbolSection;
   {
std::string SrcsSection;
     {
       llvm::raw_string_ostream SrcsOS(SrcsSection);
       for (const auto &SF : Sources)
         writeIncludeGraphNode(SF, Strings, SrcsOS);
     }
     RIFF.Chunks.push_back({riff::fourCC("srcs"), SrcsSection});
   }
 
+  llvm::SmallString<4> Cmd;
+  if (Data.CmdHash) {
+    {
+      llvm::raw_svector_ostream CmdOS(Cmd);
+      write32(llvm::hash_combine(*Data.CmdHash), CmdOS);
+    }
+    RIFF.Chunks.push_back({riff::fourCC("cmdl"), Cmd});
+  }
+
   OS << RIFF;
 }
 
 } // namespace
 
 // Defined in YAMLSerialization.cpp.
 void writeYAML(const IndexFileOut &, llvm::raw_ostream &);
 llvm::Expected<IndexFileIn> readYAML(llvm::StringRef);

clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

TEST_F(BackgroundIndexTest, UncompilableFiles) {
   EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc")));
   auto Shard = MSS.loadShard(testPath("A.cc"));
   EXPECT_THAT(*Shard->Symbols, UnorderedElementsAre(Named("foo")));
   EXPECT_THAT(Shard->Sources->keys(),
               UnorderedElementsAre("unittest:///A.cc", "unittest:///A.h",
                                    "unittest:///B.h"));
 }
 
+TEST_F(BackgroundIndexTest, CmdLineHash) {
+  MockFSProvider FS;
+  llvm::StringMap<std::string> Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(Context::empty(), FS, CDB,
+                      [&](llvm::StringRef) { return &MSS; });
+
+  tooling::CompileCommand Cmd;
+  FS.Files[testPath("A.cc")] = "#include \"A.h\"";
+  FS.Files[testPath("A.h")] = "";
+  Cmd.Filename = "../A.cc";
+  Cmd.Directory = testPath("build");
+  Cmd.CommandLine = {"clang++", "../A.cc"};
+  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  // Make sure we only store the shard for main file.
+  EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h")));
+  size_t HashInitial = *MSS.loadShard(testPath("A.cc"))->CmdHash;
+  EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->CmdHash);
+
+  // FIXME: Changing compile commands should be enough to invalidate the cache.
+  FS.Files[testPath("A.cc")] = " ";
+  Cmd.CommandLine = {"clang++", "../A.cc", "-Dfoo"};
+  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h")));
+  size_t HashSecondary = *MSS.loadShard(testPath("A.cc"))->CmdHash;
+  EXPECT_NE(HashInitial, HashSecondary);
+  EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->CmdHash);
+}
+
 } // namespace clangd
 } // namespace clang