perf(benchmark): refine JMH benchmark suite#2672
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 104 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2672 +/- ##
=========================================
Coverage 92.75% 92.75%
Complexity 4073 4073
=========================================
Files 737 737
Lines 13181 13181
Branches 922 922
=========================================
Hits 12226 12226
Misses 578 578
Partials 377 377
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fbb35730a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| @Suppress("UNCHECKED_CAST") | ||
| val primaryMetric = result["primaryMetric"] as Map<String, Any> | ||
| key to (primaryMetric["score"] as Double) |
There was a problem hiding this comment.
Parse JMH scores as Numbers before comparison
When :wow-benchmarks:benchmarkCompare reads normal JMH JSON, JsonSlurper returns decimal JSON numbers as java.math.BigDecimal rather than Double (I verified this with the Groovy runtime bundled in Gradle 8.14.4). This cast therefore throws a ClassCastException as soon as it sees a typical decimal primaryMetric.score, whereas the previous Jackson parser produced Double; use the existing parseMetricNumber helper or cast through Number here.
Useful? React with 👍 / 👎.
| private fun flushDb() { | ||
| val connection: ReactiveRedisConnection = connectionFactory.reactiveConnection | ||
| try { | ||
| connection.serverCommands().flushDb().block(FLUSH_TIMEOUT) |
There was a problem hiding this comment.
Avoid flushing the default Redis database
When any Redis benchmark constructs this fixture, RedisStandaloneConfiguration() defaults to localhost DB 0, and this call flushes the entire selected database during setup and teardown. Running :wow-benchmarks:benchmarkExternal or the Redis benchmarks on a developer/CI host with a shared local Redis will wipe unrelated keys before the benchmark starts; use a dedicated database/key prefix or delete only benchmark-owned keys.
Useful? React with 👍 / 👎.
| val snapshot = SimpleSnapshot(aggregate) | ||
| snapshotRepository.save(snapshot).block() | ||
|
|
||
| val eventStream = createEventStream() |
There was a problem hiding this comment.
Use the saved aggregate ID for snapshot loads
In this setup, aggregateId is generated separately from the aggregate ID inside createEventStream() (that helper calls cartAggregateMetadata.aggregateId() again). Saving SimpleSnapshot(stateEvent) therefore stores the snapshot under stateEvent.aggregateId, while snapshotLoad() later loads with the different aggregateId field, so the benchmark measures a cache miss/empty load instead of loading the saved snapshot; build the event stream with the same aggregate ID or load by the saved one.
Useful? React with 👍 / 👎.
Summary
wow-benchmarks/PR.Testing
./gradlew benchmarkSmoke --stacktraceRisk & Compatibility
wow-benchmarks/and do not alter runtime modules.