futures-batch icon indicating copy to clipboard operation
futures-batch copied to clipboard

Avoid creating `Delay` multiple times

Open AnthonyMikh opened this issue 9 months ago • 1 comments

Creating a Delay is a relatively heavy operation which involves creating two mutexes and an Arc, so it makes sense to avoid calling Delay::new repeatedly. In this change I introduced a new boolean field which tracks if clock is actually used, and instead of creating a new Delay from scratch I call Delay::reset on existing one if possible. This should help with performance in scenarios when buffered items are more often yielded by timeout rather than hitting maximum capacity.

(BTW I do not understand why clock is pinned since Delay is Unpin anyway. Is it an oversight?)

AnthonyMikh avatar May 04 '24 15:05 AnthonyMikh

Sorry, was playing around with benchmarks and accidentally pushed to your branch. Moved the commits to a separate branch now. https://github.com/mre/futures-batch/pull/26

I think we should run the benchmarks to see if the change makes a difference. Feel free to try it on your local machine by running cargo bench on master and your branch. There should also be an automatic GitHub action being triggered when you push to this branch again. (Although I'm not 100% convinced that we'll keep it, as it might lead to false-positives.)

mre avatar May 10 '24 10:05 mre