[PATCH] D31384: [XRay] [compiler-rt] Write buffer length to FDR log before writing buffer.

Mon Mar 27 20:55:54 PDT 2017

dberris added a comment.

In https://reviews.llvm.org/D31384#711851, @kpw wrote:

> I thought about using the header as well. If we go that route, we can't decide to optimize to only flush up through the EOB without confusing readers that aren't equipped with a per buffer frame. It seemed a little odd to have FDR specific fields in the shared header struct as well although we could always fork it into two.

We can make it explicit, by reserving the bytes right away in the header as an array of char's. This way we can pack whatever data we want in the remaining bytes in the header. Since we already record the type of the log in the header, we can get away with having a bit more information that way.

> That's fine if we're designing for the common case where recording is turned on for long enough to use many buffers before flushing.
> 
> We've got 30 bits left in the bitfield (although Trace.cpp ignores the timespec struct field when reading). xray_buffer_queue.h defines the buffer size as a size_t. What do you think is a reasonable maximum to limit it? I'll change or least comment the header there to keep things honest.

I see that this is in the head of a specific buffer. I'm not opposed to that change. I think we can put the size of the buffer as size_t in the file header, and when we're writing the records in the buffer do the computation (like you're doing) to figure out the number of bytes that's valid in the specific buffer. While the current implementation just dumps the data for the full sized buffer, I agree with you there should be nothing stopping us from writing down just the bytes that matter for a specific buffer.

I don't think we should be limiting the possible size of the buffers artificially -- it's one of those things that really ought to be tunable through flags (which it currently isn't, but is something we can do later).

https://reviews.llvm.org/D31384