Skip to content
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

RUST-793 Reduce size of returned futures #417

Merged

Conversation

patrickfreed
Copy link
Contributor

@patrickfreed patrickfreed commented Aug 4, 2021

RUST-793

This PR boxes the futures returned from execute_operation in order to reduce the size of the futures eventually returned to users.

In Rust, futures are "zero cost" in the sense that they have no hidden heap allocations (i.e. everything is on the stack), and they're implemented as giant enum-like state machines (with the code between each .await point as the states) that store all the stack data for each state. Like regular Rust enums, the size of this state machine is equivalent to the size of the largest variant, so the size of our futures are equivalent to the stack space required for the await point that uses the most stack space. Additionally, due to some missing optimizations in the compiler (rust-lang/rust#62958), the size can exponentially grow with the number of nested .await points.

This explanation is over-simplified / could be inaccurate, so I highly recommend checking out this video for a better overview of futures in Rust and the problem we're dealing with.

This is a known problem with Rust async, and the general advice is to just Box::pin the branches that might use a lot of space. I spent a lot of time digging into this for the driver, and I couldn't really pinpoint any rouge branches other than the entirety of execute_operation, so I just decided to box it entirely, as most of our futures will reach it at some point. Fortunately, this did not seem to have a noticeable impact on performance. The concerning thing is that something like find_one (which calls through to find) is still pretty massive even after this boxing (~3 KiB), but I think that's just par for the course in async Rust. For most systems this won't be a problem as the stack is pretty huge.

Size of futures before boxing:

Client::with_uri_str: 3576
list_databases: 7040
list_database_names: 6912
start_session: 1920
list_databases_with_session: 7040
list_collections: 7552
list_collections next: 8
find: 12160
find_one: 13696
insert_one: 8064
insert_many: 8192
find_one_and_replace: 11008
count_documents: 10368
update_one: 9984
estimated_document_count: 8064

Sizes after:

Client::with_uri_str: 3576
list_databases: 352
list_database_names: 240
start_session: 1920
list_databases_with_session: 360
list_collections: 280
list_collections next: 8
find: 1760
find_one: 3240
insert_one: 656
insert_many: 872
find_one_and_replace: 2384
count_documents: 1744
update_one: 1672
estimated_document_count: 536

@@ -21,7 +21,7 @@ use super::CursorResponse;
pub(crate) struct Find<T> {
ns: Namespace,
filter: Option<Document>,
options: Option<FindOptions>,
options: Option<Box<FindOptions>>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

find_one was one of the bigger ones and FindOptions itself is pretty big, so I opted to box it here too for some additional reduction in size.

@patrickfreed patrickfreed marked this pull request as ready for review August 5, 2021 16:50
@patrickfreed patrickfreed merged commit 5d09bf4 into mongodb:master Aug 6, 2021
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