Skip to content

Add benchmarks for Metadata collection #1691

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

Merged
merged 5 commits into from
Nov 1, 2023

Conversation

gjcairo
Copy link
Collaborator

@gjcairo gjcairo commented Oct 30, 2023

Motivation

This PR adds benchmarks (perf tests) for the Metadata collection introduced in #1683

Modifications

Benchmarks added

Result

Benchmarks for Metadata collection are now present.

@gjcairo gjcairo requested review from glbrntt and removed request for glbrntt October 30, 2023 08:38
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Couple of small comments, looks good otherwise. Presumably we need to add a baseline for these too.

Comment on lines 59 to 54
Benchmark("Metadata - Add binary") { benchmark in
let value: [UInt8] = [1, 2, 3]
benchmark.startMeasurement()
for _ in benchmark.scaledIterations {
var metadata = Metadata()
for i in 0..<1000 {
metadata.addBinary(value, forKey: "\(i)")
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Benchmark.init has an overload with a trailing setup closure which is executed once and can return some setup data: https://swiftpackageindex.com/ordo-one/package-benchmark/1.11.2/documentation/benchmark/benchmark/init(_:configuration:closure:setup:teardown:)-8kbjd

@gjcairo gjcairo force-pushed the metadata-benchmarks branch 2 times, most recently from e277bf2 to 9a5e91e Compare October 31, 2023 11:03
@gjcairo gjcairo force-pushed the metadata-benchmarks branch from 9a5e91e to 583df4b Compare October 31, 2023 12:50
@gjcairo gjcairo marked this pull request as ready for review October 31, 2023 12:51
}
}

Benchmark("Metadata_Remove_values_for_key") { benchmark in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is what I had originally (files with underscores, benchmark names with spaces) but the check against the thresholds would always pass even when it shouldn't - I could only make it fail once I started using underscores in the names too.

@glbrntt glbrntt merged commit 765ff32 into grpc:main Nov 1, 2023
@glbrntt glbrntt added semver/none No version bump required. version/v2 Relates to v2 labels Nov 1, 2023
@gjcairo gjcairo deleted the metadata-benchmarks branch November 1, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required. version/v2 Relates to v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants