[Lldb-commits] [PATCH] D115324: Added the ability to cache the finalized symbol tables subsequent debug sessions to start faster.

[walter erquinigo via Phabricator via lldb-commits]

wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

much nicer than the first version. I'm just asking a few minor things.



================
Comment at: lldb/include/lldb/Host/FileSystem.h:147
+  ///
+  /// The path must specify a file an not a directory.
+  /// \{
----------------



================
Comment at: lldb/include/lldb/Symbol/Symbol.h:256
+  ///   and cache file size reduction. Strings are stored as uint32_t string
+  ///   table offsets in the cache data.
+  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr,
----------------
better mention that the return value means


================
Comment at: lldb/include/lldb/Symbol/Symtab.h:148
+  ///   otherwise.
+  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr,
+              bool &uuid_mismatch);
----------------
ditto


================
Comment at: lldb/include/lldb/Utility/DataFileCache.h:59
+  ///   A valid unique pointer to a memory buffer if the data is available, or
+  ///   an shared pointer that contains NULL if the data is not available.
+  std::unique_ptr<llvm::MemoryBuffer> GetCachedData(llvm::StringRef key);
----------------



================
Comment at: lldb/include/lldb/Utility/DataFileCache.h:175
+/// class helps create string tables.
+class StringTableCreator {
+public:
----------------
I'd just call this ConstStringTable for callers to know that this is using ConstString as its storage pool and that the data doesn't just go away when the object is disposed. 


================
Comment at: lldb/source/Utility/DataFileCache.cpp:58
+  else
+    consumeError(cache_or_err.takeError());
+}
----------------
could you create a new lldb log channel where this information is logged? it'll help investigating issues with the cache (if they ever happen). Same for everywhere llvm::Error's or lldb::Status messages are lost.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115324/new/

https://reviews.llvm.org/D115324