[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D115073#3173041 <https://reviews.llvm.org/D115073#3173041>, @labath wrote:

> It seems to me that the DataEncoder class is a lot more complicated than what is necessary for our existing use cases. All of them involve creating a new buffer (either empty, or with some pre-existing content), modifying it, and passing it on. A lot of the DataEncoder complexity (`IsDynamic`, `UpdateMemberVariables`, ...) stems from the fact that it wants to support writing into unowned buffers, functionality that we don't even use.

I disagree here. We //weren't// using,  but now we are using this functionality. If you look at the ELF test that I modified, not that it is using the new heap based buffer. Even though it is very easy to calculate a buffer size in advance for this, we shouldn't have to.

Another reason I believe we need to dynamically grow the buffer is: if I am about the encode an entire symbol table into a memory buffer, how would I determine the size of the buffer I need in advance? I shouldn't have to worry about this and it would make the DataEncoder class hard to use because of this. We are going to want to encode many more things for caching using DataEncoder in the future and we shouldn't have to pre-determine how much memory is needed in advance, or allocate way too much just in case, just to be able to encode the data. How large will our symbol table name index tables be? That would be hard to determine in advance. Same with indexing of debug info, which is something that I want to add soon after.

We currently need DataEncoder for two cases:

- fixing up data that isn't relocated in pre-determined buffers for variable addresses (DW_OP_addr from .o files in DWARF without dSYM cases for Apple)
- creating a new stream of data where we don't know the size in advance

In the latter case, we might also want to  first append a bunch of data, and then use the Put methods to do some fixups on data we dynamically appended to fixup offsets to data that was created later in the buffer.

So I strongly believe that we do need the functionality that I added to DataEncoder.

> If we removed that, we could make this code a lot simpler. See inline comments for one way to achieve that.

Let me know if my comments above helped explain the APIs I added. I would like to avoid having to make two calls to a encode function to pre-determine the size of the buffers that are needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073