search

private-attribution / ipa

A raw implementation of Interoperable Private Attribution
MIT License
42 stars 25 forks source link

Reduce size of some futures #1329

Closed andyleiserson closed 1 month ago

andyleiserson commented 1 month ago

My understanding is that the future for an async fn needs space for both a copy of the arguments, and any sub-futures. Since join3v calls join3, rewriting these without async fn reduces the future size by 2/3.

A related blog post: https://swatinem.de/blog/future-size/

akoshelev commented 1 month ago

yea now I remember I read this some time ago. I like this more than boxing futures - clippy should yell at us if we revert this, so we're good here

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.52%. Comparing base (9c834cc) to head (b7ee6db). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1329 +/- ## ========================================== - Coverage 93.52% 93.52% -0.01% ========================================== Files 208 208 Lines 33890 33894 +4 ========================================== + Hits 31697 31699 +2 - Misses 2193 2195 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

andyleiserson commented 1 month ago

How would we measure the effect of this?

See #1325 for more context. There is a clippy warning if the futures get too big. The immediate tangible impacts in this case were: (1) the Box::pin calls that this change removes, (2) stack overflows in shuttle tests, because shuttle defaults to a 32 kB stack. At some point large futures could also be a performance issue.