Skip to content

Conversation

@michaelwoerister
Copy link
Member

This restricts us to timestamps of at most ~78 hours after process start which seems acceptable.
The PR reduces the size of RawEvent (and thus the size of our event stream files) by 25%.

@michaelwoerister
Copy link
Member Author

Note to myself for tomorrow: Make event encoding endianess independent as part of this PR.

@michaelwoerister
Copy link
Member Author

I added two commits:

  • Store events properly in little endian format on disk.
  • Optimize the FileSerializationSink by directly copying data to the writing buffer.

The second commit makes the FileSerializationSink exactly as fast as the MmapSerializationSink in the benchmarks we have. But I only tested on Linux and I also remember that the benchmarks were no good indication of performance when used in rustc. So maybe that commit should be split out into a separate PR.

@michaelwoerister
Copy link
Member Author

michaelwoerister commented Nov 15, 2019

I went ahead and moved the last commit to a separate, less urgent PR (#88). This PR is ready for review.

Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@wesleywiser wesleywiser merged commit 6764d49 into rust-lang:master Nov 15, 2019
LittleEndian::write_u32(&mut bytes[12..], self.start_time_lower);
LittleEndian::write_u32(&mut bytes[16..], self.end_time_lower);
LittleEndian::write_u32(&mut bytes[20..], self.start_and_end_upper);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, was the "manual" version benchmarked on x64?
The only reason I can think of is that it might be slower to write six u32s, one at a time (as opposed to 3 u64 or one u128 and one u64), but that's possible to alleviate, but first serializing to larger integers and then writing them out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the two versions were almost the same with the manual one having one instruction more (and the whole fn being something 10 instructions). I don't expect there to be much difference in practice since there is more expensive stuff going (like writing this data to disk).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants