This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by kadircet on Jul 1 2019, 9:08 AM.

Event Timeline

kadircet created this revision.Jul 1 2019, 9:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 1 2019, 9:08 AM

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.

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 updated this revision to Diff 207489.Jul 2 2019, 2:18 AM
  • Store Cmdline itself instead of hash

Still LG

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

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

sammccall accepted this revision.Jul 2 2019, 3:09 AM
sammccall added inline comments.
clang-tools-extra/clangd/index/Serialization.cpp
419

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

This revision is now accepted and ready to land.Jul 2 2019, 3:09 AM
kadircet updated this revision to Diff 207578.Jul 2 2019, 9:26 AM
kadircet marked 3 inline comments as done.
  • Use an InternedCompileCommand struct rather than a pack of StringRefs.
sammccall accepted this revision.Jul 2 2019, 11:34 PM
sammccall added inline comments.
clang-tools-extra/clangd/index/Serialization.cpp
531

nit: push_back, or emplace_back(C)

590

nit: internedcmd to avoid confusion below?

kadircet updated this revision to Diff 207721.Jul 3 2019, 1:06 AM
kadircet marked 2 inline comments as done.
  • Address comments
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2019, 2:52 AM