-
Notifications
You must be signed in to change notification settings - Fork 250
feat(core): add stripped MastForest serialization #2549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
4406e14 to
9492508
Compare
7c4a99e to
c88a02a
Compare
9492508 to
9ddd236
Compare
c88a02a to
65a4972
Compare
9ddd236 to
c47eabf
Compare
65a4972 to
7dc6b6e
Compare
c47eabf to
4f4704c
Compare
Add support for serializing MastForest without debug information, producing smaller output files for production deployment. Changes: - Update header format: MAST (4 bytes) + FLAGS (1 byte) + VERSION (3 bytes) - Add FLAG_STRIPPED (0x01) to indicate debug info is omitted - Add MastForest::write_stripped() public method - Deserializer auto-detects format and creates empty DebugInfo when stripped - Reject unknown flags (reserved bits must be zero)
4f4704c to
9f1e016
Compare
plafer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, LGTM!
bobbinth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thank you!
One thought for the future: stripping debug info removes most but not all debug-related info from the forest. Specifically, what is not removed are the inner values of Assert, U32assert2, and MpVerify operations. These contain error codes that probably are not super useful if we don't care about debugging.
There isn't a simple way to get rid of these though because operation serialization is handled pretty far deep in the stack.
Separately (and also not for this PR), I do think we should rename MastForest::strip_decorators() into something more like MastForest::clear_debug_info() because this method does more than just strip decorators.
| // node & decorator counts | ||
| target.write_usize(self.nodes.len()); | ||
| // Expected to be used in #2504. Remove if this issue is resolved without using. | ||
| target.write_usize(self.debug_info.num_decorators()); | ||
| target.write_usize(if stripped { 0 } else { self.debug_info.num_decorators() }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for this PR, but I'm still not sure we need to serialize these values.
* refactor: rename strip_decorators to clear_debug_info The method clears the entire DebugInfo, not just decorators. * chore: Changelog * chore: fix CHANGELOG entries for #2549 and #2554 - Add method name `write_stripped()` to #2549 entry - Fix typo in #2554 entry: clear_debug_info (with underscore)
On top of #2466 #2469 #2470. Adds support for serializing MastForest without debug information, producing smaller output files for production deployment.
FLAG_STRIPPED (0x01)to indicate debug info is omittedMastForest::write_stripped()public method, redirect to a proxy newtype (for type safety).DebugInfowhen strippedTogether with the above PRs this is stacked on, closes #1580.
Note: this complements rather than replaces the in-memory decorator stripping from #2108, which remains useful for improving execution and proving performance.