search

sourcegraph / conc

Better structured concurrency for go
https://about.sourcegraph.com/blog/building-conc-better-structured-concurrency-for-go
MIT License
9.05k stars 312 forks source link

Make result order deterministic #126

Closed camdencheek closed 9 months ago

camdencheek commented 10 months ago

This PR makes the order of results in a Result.*Pool deterministic so that the order of the result slice corresponds with the order of tasks submitted. As an example of why this would be useful, it makes it easy to rewrite iter.Map in terms of ResultPool. Additionally, it's a generally nice and intuitive property to be able to match the index of the result slice with the index of the input slice.

Comments inline.

codecov-commenter commented 10 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (30a99cd) 99.31% compared to head (e1e9af2) 98.94%.

Files Patch % Lines
pool/result_pool.go 94.73% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #126 +/- ## ========================================== - Coverage 99.31% 98.94% -0.38% ========================================== Files 12 12 Lines 441 474 +33 ========================================== + Hits 438 469 +31 - Misses 3 4 +1 - Partials 0 1 +1 ```

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

camdencheek commented 9 months ago

this does add an extra set of lock/unlock on the mutex

Yes, it does. It would be possible to use an atomic integer for reserving a slot, but it adds more complexity of the implementation and I wouldn't expect these to be a high-contention since in most cases, the task being done in a goroutine is large. We can do some benchmarking and improve perf later if the difference turns out to be significant.

sagikazarmark commented 4 months ago

Any plans to tag a new release with this change?