[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 ↗

(On Diff #207343)

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

48 ↗

(On Diff #207489)

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

[sammccall]

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

418 ↗

(On Diff #207489)

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

531 ↗

(On Diff #207578)

nit: push_back, or emplace_back(C)

590 ↗

(On Diff #207578)

nit: internedcmd to avoid confusion below?

[kadircet]

• Address comments

clang-tools-extra/trunk/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.Cmd = Index.Cmd.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;
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.Cmd = Inputs.CompileCommand;
   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/trunk/clangd/index/Serialization.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RIFF_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RIFF_H
 
 #include "Headers.h"
 #include "Index.h"
 #include "index/Symbol.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
 namespace clangd {
 
 enum class IndexFileFormat {
   RIFF, // Versioned binary format, suitable for production use.
   YAML, // Human-readable format, suitable for experiments and debugging.
 };
 
 // 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;
+  // This contains only the Directory and CommandLine.
+  llvm::Optional<tooling::CompileCommand> Cmd;
 };
 // 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 tooling::CompileCommand *Cmd = 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),
+        Cmd(I.Cmd ? I.Cmd.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/trunk/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 "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+#include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 llvm::Error makeError(const llvm::Twine &Msg) {
   return llvm::make_error<llvm::StringError>(Msg,
                                              llvm::inconvertibleErrorCode());
 }

 Relation readRelation(Reader &Data) {
   SymbolID Subject = Data.consumeID();
   index::SymbolRole Predicate =
       relationKindToSymbolRole(static_cast<RelationKind>(Data.consume8()));
   SymbolID Object = Data.consumeID();
   return {Subject, Predicate, Object};
 }
 
+struct InternedCompileCommand {
+  llvm::StringRef Directory;
+  std::vector<llvm::StringRef> CommandLine;
+};
+
+void writeCompileCommand(const InternedCompileCommand &Cmd,
+                         const StringTableOut &Strings,
+                         llvm::raw_ostream &CmdOS) {
+  writeVar(Strings.index(Cmd.Directory), CmdOS);
+  writeVar(Cmd.CommandLine.size(), CmdOS);
+  for (llvm::StringRef C : Cmd.CommandLine)
+    writeVar(Strings.index(C), CmdOS);
+}
+
+InternedCompileCommand
+readCompileCommand(Reader CmdReader, llvm::ArrayRef<llvm::StringRef> Strings) {
+  InternedCompileCommand Cmd;
+  Cmd.Directory = CmdReader.consumeString(Strings);
+  Cmd.CommandLine.resize(CmdReader.consumeVar());
+  for (llvm::StringRef &C : Cmd.CommandLine)
+    C = CmdReader.consumeString(Strings);
+  return Cmd;
+}
+
 // FILE ENCODING
 // 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
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 CmdReader(Chunks.lookup("cmdl"));
+    if (CmdReader.err())
+      return makeError("malformed or truncated commandline section");
+    InternedCompileCommand Cmd =
+        readCompileCommand(CmdReader, Strings->Strings);
+    Result.Cmd.emplace();
+    Result.Cmd->Directory = Cmd.Directory;
+    Result.Cmd->CommandLine.reserve(Cmd.CommandLine.size());
+    for (llvm::StringRef C : Cmd.CommandLine)
+      Result.Cmd->CommandLine.emplace_back(C);
+  }
   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) {
   std::vector<Relation> Relations;
   if (Data.Relations) {
     for (const auto &Relation : *Data.Relations) {
       Relations.emplace_back(Relation);
       // No strings to be interned in relations.
     }
   }
 
+  InternedCompileCommand InternedCmd;
+  if (Data.Cmd) {
+    InternedCmd.CommandLine.reserve(Data.Cmd->CommandLine.size());
+    InternedCmd.Directory = Data.Cmd->Directory;
+    Strings.intern(InternedCmd.Directory);
+    for (llvm::StringRef C : Data.Cmd->CommandLine) {
+      InternedCmd.CommandLine.emplace_back(C);
+      Strings.intern(InternedCmd.CommandLine.back());
+    }
+  }
+
   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});
   }
 
+  std::string CmdlSection;
+  if (Data.Cmd) {
+    {
+      llvm::raw_string_ostream CmdOS(CmdlSection);
+      writeCompileCommand(InternedCmd, Strings, CmdOS);
+    }
+    RIFF.Chunks.push_back({riff::fourCC("cmdl"), CmdlSection});
+  }
+
   OS << RIFF;
 }
 
 } // namespace
 
 // Defined in YAMLSerialization.cpp.
 void writeYAML(const IndexFileOut &, llvm::raw_ostream &);
 llvm::Expected<IndexFileIn> readYAML(llvm::StringRef);

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

 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/Background.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <thread>
 
 using ::testing::_;
 using ::testing::AllOf;
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, /*FallbackFlags=*/{},
+                 /*ResourceDir=*/std::string(""));
+  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", "-fsyntax-only"};
+  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h")));
+  // Make sure we only store the Cmd for main file.
+  EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
+
+  {
+    tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
+    EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
+    EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
+  }
+
+  // FIXME: Changing compile commands should be enough to invalidate the cache.
+  FS.Files[testPath("A.cc")] = " ";
+  Cmd.CommandLine = {"clang++", "../A.cc", "-Dfoo", "-fsyntax-only"};
+  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
+
+  {
+    tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
+    EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
+    EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
+  }
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp

 //===-- SerializationTests.cpp - Binary and YAML serialization unit tests -===//
 //
 // 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 "index/Index.h"
 #include "index/Serialization.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Pair;
Out.Sources = &Sources;
     auto IGNDeserialized = In->Sources->lookup(IGN.URI);
     EXPECT_EQ(IGNDeserialized.Digest, IGN.Digest);
     EXPECT_EQ(IGNDeserialized.DirectIncludes, IGN.DirectIncludes);
     EXPECT_EQ(IGNDeserialized.URI, IGN.URI);
     EXPECT_EQ(IGNDeserialized.IsTU, IGN.IsTU);
   }
 }
 
+TEST(SerializationTest, CmdlTest) {
+  auto In = readIndexFile(YAML);
+  EXPECT_TRUE(bool(In)) << In.takeError();
+
+  tooling::CompileCommand Cmd;
+  Cmd.Directory = "testdir";
+  Cmd.CommandLine.push_back("cmd1");
+  Cmd.CommandLine.push_back("cmd2");
+  Cmd.Filename = "ignored";
+  Cmd.Heuristic = "ignored";
+  Cmd.Output = "ignored";
+
+  IndexFileOut Out(*In);
+  Out.Format = IndexFileFormat::RIFF;
+  Out.Cmd = &Cmd;
+  {
+    std::string Serialized = llvm::to_string(Out);
+
+    auto In = readIndexFile(Serialized);
+    ASSERT_TRUE(bool(In)) << In.takeError();
+    ASSERT_TRUE(In->Cmd);
+
+    const tooling::CompileCommand &SerializedCmd = In->Cmd.getValue();
+    EXPECT_EQ(SerializedCmd.CommandLine, Cmd.CommandLine);
+    EXPECT_EQ(SerializedCmd.Directory, Cmd.Directory);
+    EXPECT_NE(SerializedCmd.Filename, Cmd.Filename);
+    EXPECT_NE(SerializedCmd.Heuristic, Cmd.Heuristic);
+    EXPECT_NE(SerializedCmd.Output, Cmd.Output);
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang