[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D39436#912902, @hintonda wrote:

> In https://reviews.llvm.org/D39436#912829, @clayborg wrote:
>
> > In https://reviews.llvm.org/D39436#912828, @zturner wrote:
> >
> > > In https://reviews.llvm.org/D39436#912810, @clayborg wrote:
> > >
> > > > I was unhappy when we went over two pointers for a FileSpec when m_syntax was added due to the extra size. Anything we can do to make this smaller would be great, so the type on the enum would work, but as you say the alignment will nullify that. The two ConstString members contain a pointer which isn't aligned so we can't use any bits from the low end of the pointer. Are there any classes that take advantage of high bits in pointers? Most if not all OS's don't use the entire 64 bit address space... It would be great to get lldb_private::FileSpec down to just 2 pointers again.
> > >
> > >
> > > `ConstString` doesn't *currently* contain aligned pointers, but there's no reason we couldn't make it contain aligned pointers.  Then we could use `llvm::PointerUnion`.
> >
> >
> > I would be fine with that.
> >
> > > That said, I want to state again that I think this change is the wrong direction.  I don't think we need this functionality in `FileSpec`, or even in another class.  I think it is better served in the script.
> >
> > Agreed as well. I do like the idea of source path regexes, but only if we have a real need to add it to the API.
>
>
> I haven't had time to really look into this, but it seems that maintaining two independent strings, one for directory and one for basename, is just for convenience.   We could easily keep it in a single string with an index to basename.  When the directory portion is updated, e.g., resolved, etc., we just overwrite the string and adjust the index.  And adjust accessors as needed.


The reason it's two strings is for memory efficiency and de-duplication.  Suppose you make `FileSpec` instances from  `foo/bar/baz/` and `foo/bar/buzz`.  This gets separated into 4 instances of `ConstString`.  `foo/bar`, `baz`, `foo/bar`, and `buzz`.  Because `ConstString`s are pooled, there's actually only 3 strings here.  `foo/bar`, `baz`, and `buzz`.

It probably doesn't seem like a lot, but over the course of thousands and thousands of files (which debuggers often examine and which often share a common parent directory) this is a large memory savings.


https://reviews.llvm.org/D39436