perf(core): cut per-row overhead on the result-mapping hot path#277
Merged
Conversation
Reduce work done while materializing result sets: - QueryImpl: resolve the JDBC column reader once per column per query (cached per target type via ClassValue) instead of re-running the target-type switch for every column of every row. Reference-returning getters skip the now-redundant wasNull() probe. - WeakInterner: key entities by concrete type then primary key in a two-level map, avoiding a Ref allocation per interned entity while keeping the same identity semantics. - RecordMapper: reuse the compiled plan's cached parameterTypes length instead of re-walking the record structure on every query, and short-circuit the dirty-tracking lookup for non-transactional reads.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Reduces work done while materializing result sets, on the paths that run for every row of every query. Behavior is unchanged; the full storm-core suite (2305 tests) passes.
Changes
QueryImpl— resolve the column reader once per column, not per cell.The per-column target-type dispatch (a large
switchover the targetClass) previously ran for every column of every row. It now resolves aColumnReaderonce per column per query, cached per target type via aClassValue, and reuses it for every row. Reference-returning JDBC getters (String,BigDecimal, temporal types, …) drop the redundantwasNull()probe since they already yieldnullfor SQL NULL; primitive getters keep it.WeakInterner— key entities by type + primary key, noRefallocation.Entity interning previously built a
Refper lookup and store. It now uses a two-levelClass → (pk → weak ref)map keyed by the primary key value already in hand, avoiding the per-entityRefallocation while preserving identity semantics (same type + pk = same canonical instance).ReferenceQueuecleanup is retained via a two-argument remove.RecordMapper— reuse cached plan metadata, short-circuit the cache lookup.The parameter-count check reuses the compiled plan's cached
parameterTypes().lengthinstead of re-walking the record structure on every query. The entity-cache resolution short-circuits the dirty-tracking mode lookup for non-transactional / non-entity reads, so it is never computed when it cannot apply.Verification
mvn -pl storm-core test— 2305 tests, 0 failures/errors (JDK 21). Directly-affected suites (WeakInternerTest,QueryImplTypeMappingIntegrationTest,EntityCacheImplTest) all green.