feat(pb): box stream NodeBody to reduce stack memory usage #19911
Conversation
Signed-off-by: xxchan <[email protected]>
This reverts commit a2e53a5.
Signed-off-by: xxchan <[email protected]>
| fn test_size() { | ||
| use static_assertions::const_assert_eq; | ||
|
|
||
| const_assert_eq!(std::mem::size_of::<NodeBody>(), 16); |
| @@ -112,6 +112,57 @@ fn main() -> Result<(), Box<dyn std::error::Error>> { | |||
| "#[derive(::enum_as_inner::EnumAsInner)]", | |||
| ) | |||
| .btree_map(btree_map_paths) | |||
| // node body is a very large enum, so we box it to avoid stack overflow. | |||
| // TODO: ideally we should box all enum variants automatically https://github.com/tokio-rs/prost/issues/1209 | |||
| .boxed(".stream_plan.StreamNode.node_body.source") | |||
There was a problem hiding this comment.
Why not .boxed(".stream_plan.StreamNode")?
There was a problem hiding this comment.
Sounds can also fix the stackoverflow. 🤔 But the enum itself still wastes memory.
There was a problem hiding this comment.
Listing all possible variants here seems verbose. Is this the only approach to achieve the goal?
There was a problem hiding this comment.
can implement this upstream tokio-rs/prost#1209
Signed-off-by: xxchan <[email protected]>
| .boxed(".stream_plan.StreamNode.node_body.source") | ||
| .boxed(".stream_plan.StreamNode.node_body.project") | ||
| .boxed(".stream_plan.StreamNode.node_body.filter") | ||
| .boxed(".stream_plan.StreamNode.node_body.materialize") |
There was a problem hiding this comment.
Shall we box the batch plan node as well?
There was a problem hiding this comment.
batch node body was 680 bytes. After boxing Udf in Expr, it's 464 bytes. Sounds acceptable
There was a problem hiding this comment.
LocalLookupJoinNode is the largest
| @@ -112,6 +112,57 @@ fn main() -> Result<(), Box<dyn std::error::Error>> { | |||
| "#[derive(::enum_as_inner::EnumAsInner)]", | |||
| ) | |||
| .btree_map(btree_map_paths) | |||
| // node body is a very large enum, so we box it to avoid stack overflow. | |||
| // TODO: ideally we should box all enum variants automatically https://github.com/tokio-rs/prost/issues/1209 | |||
| .boxed(".stream_plan.StreamNode.node_body.source") | |||
There was a problem hiding this comment.
Listing all possible variants here seems verbose. Is this the only approach to achieve the goal?
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
fix #19910 See analysis in the issue
also revert #19695
Checklist
Documentation
Release note