[Lldb-commits] [PATCH] D151765: [lldb] Introduce the FileSpecBuilder abstraction

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The idea behind FileSpec containing two ConstStrings, one for the directory and one for he filename is for lookup performance when searching thousands of line tables for a given file and line when setting breakpoints.

Currently we pass in a FileSpec + line number and we expect fast searching. This is currently done by asking each CompileUnit to find a matching file in its support files array and to return a matching index. If just the filename is filled in, then we compare the just the ConstString for m_filename which is really quick for ConstString values. Then we compare all or part of the m_directory if the path is a full path or if it is relative. But at least the filename comparison is very fast. If we get a valid index back, then we know to search the compile unit's full line table for any matching entries by looking for any line entries with a matching file index. IF the file index is invalid, we don't materialize the line table at all for a compile unit.

Maybe we want the notion of a "FileSpec" (which could contain a llvm::Vector class) and a ConstFileSpec (which would contain two ConstString objects like FileSpec currently has). And we make anyone that needs quick searching, like the support files for a compile unit, use the new ConstFileSpec class.

If you change anything in the FileSpec class, please test setting breakpoints with a really large codebase to ensure we don't regress performance.

If we go the FileSpecBuilder route, we probably want to remove all FileSpec path modification methods and force people to use this class by converting a FileSpec to a FileSpecBuilder, manipulating the path with the FileSpecBuilder calls, and then extracting the FileSpec object from the FileSpecBuilder at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151765