[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/trunk/clangd/index/Index.h:71
+  // via llvm::StringRef.
+  const char *FileURI = "";
 };
----------------
alexfh wrote:
> If the users of the SymbolLocation have a way to get a common "context", you could keep a list of filenames in this context and just store an index into this list (which could be even shorter). If the structure is used where no context is available, you could still keep a list of StringRefs somewhere (probably close to where the string data pointed by the StringRefs lives) and keep a `StringRef *` here. At a cost of one indirection you'd avoid calling strlen.
> 
> Not sure what trade-off is better for this particular use case though.
`SymbolLocation` is a public interface, and the underlying data is stored in arena (which is not visible to clients), so clients don't know any `Context` except `SymbolLocation`.

 Using `StringRef *` is an interesting idea -- keeping a list of `StringRefs` needs extra space, the number should be relatively small (only the number of different files), so we will spend  extra `<num of files> * sizeof(StringRef)` bytes (for llvm, ~128 KB memory) , compared with the raw pointer solution.


Repository:
  rL LLVM

https://reviews.llvm.org/D53427