Fix NIOAsyncChannel allocation benchmarks
#2622
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FranzBusch
commented
Jan 9, 2024
| try await self._storage.yield(contentsOf: CollectionOfOne(element)) | ||
| try await self._storage.yield(element: element) |
There was a problem hiding this comment.
The fix here is quite simple. The problem was that we lost the fast path for single element writes in some refactoring. This PR duplicates the write path and specialises one of them for single element writes to call the optimised delegate method of the writer.
FranzBusch
force-pushed
the
fb-async-channel-benchmark-fix
branch
from
1abaab4
to
5ff86ef
Compare
dnadoba
approved these changes
Jan 9, 2024
hassila
reviewed
Jan 11, 2024
| configuration: .init( | ||
| metrics: defaultMetrics, | ||
| timeUnits: .milliseconds, | ||
| scalingFactor: .mega, |
There was a problem hiding this comment.
If you update to Benchmark 1.22.0 you can now add some slack to the p90 check for malloc counts, this should do the trick (allowing for a +-100 mallocs);
Suggested change
| scalingFactor: .mega, | |
| scalingFactor: .mega, | |
| thresholds: [.mallocCountTotal : .init(absolute: [.p90 : 100])], |
FranzBusch
force-pushed
the
fb-async-channel-benchmark-fix
branch
2 times, most recently
from
3312324
to
ad67033
Compare
dnadoba
approved these changes
Jan 11, 2024
FranzBusch
force-pushed
the
fb-async-channel-benchmark-fix
branch
from
ad67033
to
9775ef7
Compare
# Motivation We had to disable the benchmarks since they regressed without us noticing and they appear to be flaky. # Modification This PR fixes the allocation regression and tries to re-enable them.
FranzBusch
force-pushed
the
fb-async-channel-benchmark-fix
branch
from
9775ef7
to
02efe9d
Compare
glbrntt
added
the
needs-no-version-bump
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
We had to disable the benchmarks since they regressed without us noticing and they appear to be flaky.
Modification
This PR fixes the allocation regression and tries to re-enable them.