[memprof] Omit the key/data lengths for the frame table #89711
Conversation
The frame table has constant key/data lengths, so we don't need to serialize or deserialize them for every key-data pair. Omitting the key/data lengths saves 0.21% of the indexed MemProf file size. Note that it's OK to change the format because Version2 is still under development.
| OnDiskChainedHashTableGenerator<memprof::FrameWriterTrait> | ||
| FrameTableGenerator; | ||
| for (auto &[FrameId, Frame] : MemProfFrameData) { | ||
| // Insert the key (frame id) and value (frame contents). | ||
| FrameTableGenerator.insert(FrameId, Frame); | ||
| FrameTableGenerator.insert(FrameId, Frame, FrameWriter); |
There was a problem hiding this comment.
Are there changes needed in this patch to the insert() and Emit() methods of OnDiskChainedHashTableGenerator to take the writer trait?
There was a problem hiding this comment.
Are there changes needed in this patch to the insert() and Emit() methods of OnDiskChainedHashTableGenerator to take the writer trait?
Yes, FrameWriter carries the version, so we need to pass FrameWriter to both insert and Emit.
There was a problem hiding this comment.
Ok, I think that part is missing from the PR?
|
I'm going to discard this PR. We would need to route the writer trait to even more places (than present in this patch) to correctly invoke @teresajohnson Thank you for reviewing this patch! |
The frame table has constant key/data lengths, so we don't need to
serialize or deserialize them for every key-data pair. Omitting the
key/data lengths saves 0.21% of the indexed MemProf file size.
Note that it's OK to change the format because Version2 is still under
development.