labath added a comment.
Maybe it's good for starting a discussion, but I am surprised that we would
need to cache symtab information. This is a fairly simple format that needs to
be read at application runtime as well, so I am surprised that reading it from
the cache is substantially faster than doing it from the object file. What is
the slow part there? Is it demangling by any chance?
Now to the discussion. Overall, I think this is an interesting feature, but I
do have some remarks/questions:
- I don't think that "lldb module cache" is very good name for this feature as
we already have the "platform module cache" feature (see `Target/ModuleCache.h`
and its callers), which deals with downloading (and caching) modules (object
files) from remote systems. And then there's the clang module cache, of
course.. (and the llvm modules, but we fortunately don't cache those). It would
be better if we chose a slightly less overloaded name. Maybe "index cache" ? (I
assume you will also want to store dwarf indexes there)
- I don't see anything which would ensure cache consistency for the case where
multiple debugger instances try to cache the same module simultaneously. What's
the plan for that?
- Have you considered the building upon the caching infrastructure in llvm
(`llvm/Caching.h`, `llvm/CachePruning.h`)? I believe it already has some size
limiting features and multiprocess synchronization built-in, so using it would
take care of the previous item, and of @wallace's size limit request.
- it's moderately strange to be using the gsym file writer for this purpose
I haven't looked at the code in detail, but I noted that the filesystem
changes, and some of the unrelated formatting cleanups could be put into
separate patches.
================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2722
m_symtab_up = std::make_unique<Symtab>(symtab->GetObjectFile());
+ if (m_symtab_up->LoadFromCache())
+ return m_symtab_up.get();
----------------
Can we make it so that we don't need to add these bits to every object file
implementation? Maybe create a protected `CreateSymtab` function, which would
get only called if the Symtab really needs to be created?
================
Comment at: lldb/source/Symbol/Symbol.cpp:645
+ file.writeU16(m_type_data);
+ file.writeU16((&m_type_data)[1]);
+ m_mangled.Encode(file);
----------------
It would be (much) better to put the bitfields in an (anonymous) union that can
be accessed both as a uint16_t and as individual bitfields.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113789/new/
https://reviews.llvm.org/D113789
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
