perf(core): optimize benchmarked hot paths#2671
Conversation
Merge critical JMH service provider files so benchmark forks load the same Jackson and id generator providers as the runtime classpath. Stabilize benchmark fixtures that previously accumulated unbounded in-memory state or buffered sink values, and use valid Wow ids/snapshot payloads in JMH inputs. Optimize the example cart state sourcing path by mutating the backing item list instead of copying it on every added item event, reducing aggregate recovery allocation for long event streams.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 161 |
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❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2671 +/- ##
============================================
- Coverage 92.75% 92.09% -0.67%
- Complexity 4073 4105 +32
============================================
Files 737 738 +1
Lines 13181 13518 +337
Branches 922 989 +67
============================================
+ Hits 12226 12449 +223
- Misses 578 665 +87
- Partials 377 404 +27
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:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d58d7040a
ℹ️ 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".
| lazyAttributes ?: ConcurrentHashMap<String, Any>(ATTRIBUTE_MAP_INITIAL_CAPACITY).also { | ||
| copyFieldAttributesTo(it) | ||
| lazyAttributes = it |
There was a problem hiding this comment.
When a SimpleServerCommandExchange is shared across scheduler threads, a first call to attributes can snapshot the volatile field-backed attributes and install lazyAttributes while another thread is concurrently calling setError, setEventStream, or another typed setter. If that setter observes lazyAttributes == null and writes only the field just after copyFieldAttributesTo, all later getAttribute calls read only the map and the newly written attribute is effectively lost; the previous ConcurrentHashMap backing did not have this race. Please guard field-backed writes/removes with the same synchronization used for materialization, or re-check/copy fields after installing the map.
Useful? React with 👍 / 👎.
| val primaryMetric = result["primaryMetric"] as Map<String, Any> | ||
| key to (primaryMetric["score"] as Double) |
There was a problem hiding this comment.
When benchmarkCompare runs against normal JMH JSON, JsonSlurper materializes decimal scores as BigDecimal (and sometimes other Number types), so this as Double cast throws a ClassCastException before any comparison can be printed. The same unchecked Double casts remain in generateBenchmarkReport; reuse the existing parseMetricNumber/Number.toDouble() path here so the reporting tasks can consume JMH output produced by the new parser.
Useful? React with 👍 / 👎.
| connectionFactory = LettuceConnectionFactory(redisConfig, lettuceClientConfiguration) | ||
| connectionFactory.afterPropertiesSet() | ||
| redisTemplate = ReactiveStringRedisTemplate(connectionFactory) | ||
| flushDb() |
There was a problem hiding this comment.
Running any Redis benchmark now constructs this fixture with RedisStandaloneConfiguration() defaults, which points at localhost database 0, and immediately calls FLUSHDB (again on close below). In a developer or shared benchmark host that already has Redis data in DB 0, simply starting benchmarkExternal deletes unrelated keys; use an isolated database/container or only clear the benchmark key prefix instead.
Useful? React with 👍 / 👎.
1 participant
Summary
wow-benchmarksGradle logic into focused reporting and JMH packaging scripts..gitignorerule that excludeddocs/superpowers/.Changes
RedisScriptsand reuses script text in Redis prepare and event-store paths.docs/superpowers/files to be tracked by removing the local ignore rule.Testing
./gradlew :wow-core:check :wow-redis:check :example-domain:check :wow-benchmarks:benchmarkSmoke && git diff --check./gradlew :wow-benchmarks:benchmarkSmoke --rerun-tasksRisk & Compatibility
Review Notes