[WIP] Reduce per-column overhead for ResultSet bulk reads - #2934
Draft
machavan wants to merge 2 commits into
Draft
[WIP] Reduce per-column overhead for ResultSet bulk reads#2934machavan wants to merge 2 commits into
machavan wants to merge 2 commits into
Conversation
- Cache and reuse ServerDTVImpl instances across rows (avoid 601M allocs) - Reuse TDSReaderMark objects via markInto() (avoid 601M allocs) - Fast-path readUnsignedByte() with inline payload check - Guard loggerExternal.entering/exiting in hot-path getters - Optimize discardCurrentRow() loop bounds - Fast-path small CHAR/VARCHAR string reads bypassing SimpleInputStream
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2934 +/- ##
============================================
- Coverage 60.43% 59.44% -1.00%
+ Complexity 4916 4865 -51
============================================
Files 151 151
Lines 35132 35236 +104
Branches 5873 5900 +27
============================================
- Hits 21231 20945 -286
- Misses 11052 11451 +399
+ Partials 2849 2840 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
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.
Summary
When reading large, wide result sets (e.g., 1M rows × 600 columns), profiling reveals that per-column overhead — object allocations, no-op logging calls, and intermediate stream construction — accumulates significantly across hundreds of millions of column accesses. The driver's lazy per-column reading architecture is well-suited for sparse column access patterns, but when applications read every column of every row, there are opportunities to reduce repeated work without changing that architecture.
This PR reduces that overhead through six targeted, low-risk changes. All changes preserve existing behavioral semantics — same values returned, same exceptions thrown, same API contracts honored.
Files changed: 3 | Insertions: 140 | Deletions: 44
Changes
A. Cache and reuse
ServerDTVImplinstances (dtv.java)Problem: Every column on every row allocates a
new ServerDTVImpl()when its value is first accessed (viaDTV.skipValue(),DTV.getValue(), orDTV.initFromCompressedNull()). When the row is discarded,DTV.clear()setsimpl = null, and the object becomes garbage. For wide result sets this means millions of short-lived allocations that pressure the young generation GC.Fix:
DTV.clear()now stashes the outgoingServerDTVImplin acachedServerImplfield instead of discarding it. The new helpergetOrCreateServerDTVImpl()returns the cached instance (after callingreinit()to reset its fields) if available, or creates a new one otherwise. This converts millions of allocations into at most one per column position, reused across all rows.Risk: Low.
reinit()explicitly zeros all fields to the same state as a fresh constructor. The cache is per-DTV (per-column), so there are no cross-column or cross-row aliasing concerns.AppDTVImplinstances (from updatable result sets) are not cached — onlyServerDTVImpl.B. Reuse
TDSReaderMarkobjects (dtv.java,IOBuffer.java)Problem:
ServerDTVImpl.getValuePrep()callstdsReader.mark()for every column value it encounters, which allocatesnew TDSReaderMark(packet, offset). Combined with Change A, this adds millions of additional allocations of a 2-field object that is only used to save/restore a position in the TDS packet stream.Fix:
TDSReaderMarkfields changed fromfinalto mutable, and a no-arg constructor +set(packet, offset)method added.TDSReader.markInto(TDSReaderMark)added alongside the existingmark()— writes into an existing mark object instead of creating a new one.ServerDTVImplnow pre-allocates oneTDSReaderMarkat construction time and reuses it viamarkInto(). AhasValueMarkboolean replaces thevalueMark == nullchecks.mark()method is unchanged and still used by all other callers (scroll windows, fetch buffer reset, LOB streams, etc.).Risk: Low. The mutable
TDSReaderMarkis private toServerDTVImpland never exposed externally. All othermark()callsites are untouched. ThesetPositionAfterStreamed()path (adaptive streaming) continues to use the allocatingmark().C. Fast-path
readUnsignedByte()(IOBuffer.java)Problem:
TDSReader.readUnsignedByte()unconditionally callsensurePayload()(a method call) before reading a single byte. This method is called as the length-prefix reader for everyBYTELENTYPEcolumn (e.g.,char(1),varchar(64), nullable int/float/decimal). In the vast majority of cases the current packet still has data — theensurePayload()call is a no-op.Fix: Add an inline fast-path check
payloadOffset < currentPacket.payloadLengthbefore theensurePayload()call, matching the pattern already used byreadInt(),readShort(), andreadLong():Risk: Very low. This is a pure performance optimization with identical semantics. The slow path is preserved for packet-boundary cases.
D. Guard
loggerExternal.entering/exitingin hot-path getters (SQLServerResultSet.java)Problem: Every ResultSet getter (
getInt,getLong,getString,getDouble,getBigDecimal,getTimestamp,wasNull) unconditionally callsloggerExternal.entering(getClassNameLogging(), ...)andloggerExternal.exiting(getClassNameLogging(), ...). Even when logging is disabled (the normal case), each call involves:getClassNameLogging()method callint columnIndex→Integer)java.util.logging.Logger.entering()which internally callsisLoggable(Level.FINER)For wide result sets this adds up to billions of no-op logging calls.
Fix: Wrap
entering/exitingcalls inif (loggerExternal.isLoggable(java.util.logging.Level.FINER))guards. This is the same pattern already used by some other methods in the class (e.g.,getBigDecimal(int, int),getTimestamp(int, Calendar)).Guarded methods:
getInt(int),getInt(String),getLong(int),getLong(String),getDouble(int),getDouble(String),getString(int),getString(String),getBigDecimal(int),getBigDecimal(String),getTimestamp(int),getTimestamp(String),wasNull().Risk: None. Logging behavior is identical — the guard is the same check that
Logger.entering/exitingperforms internally. When logging is enabled, the same messages are emitted.E. Optimize
discardCurrentRow()cleanup loop (SQLServerResultSet.java)Problem: When
next()is called,discardCurrentRow()iterates through all previously-accessed columns callingcolumn.clear()on each one individually. For a wide result set where all columns were read, this is hundreds ofclear()calls per row.Fix: Use
Math.min(lastColumnIndex, columns.length + 1)to bound the clear loop correctly, avoiding any potential out-of-bounds iteration. The core loop remains — clearing is necessary to triggerServerDTVImplcaching (Change A) — but the bounds are tighter.Risk: Very low. The loop performs the same work, with slightly cleaner bounds.
F. Fast-path small string reads without
SimpleInputStream(dtv.java)Problem: When
getString()is called on aCHAR,VARCHAR,NCHAR, orNVARCHARcolumn,ServerDTVImpl.getValue()creates anew SimpleInputStream(tdsReader, valueLength, ...), which is then passed toDDC.convertStreamToObject(). That method callsstream.getBytes()(allocatesbyte[valueLength], reads, closes the stream) →new String(bytes, charset). TheSimpleInputStreamcreation involves:BaseInputStreamconstructor (field init, conditional mark)getBytes()→new byte[payloadLength]allocation +read()loop +close()close()→ skip remaining +closeHelper()(null out fields)For wide result sets with many string columns, this means millions of
SimpleInputStreamallocations + millions ofbyte[]allocations where the stream is immediately consumed and discarded.Fix: For
CHAR/VARCHAR/NCHAR/NVARCHARcolumns where:valueLength > 0 && valueLength <= 4000(not a large value)streamGetterArgs.streamType == StreamType.NONE(not a streaming access)!streamGetterArgs.isAdaptive(not adaptive buffering)jdbcType.isTextual()(target type is a character type, i.e.,getString()path)...read bytes directly from
TDSReaderinto abyte[]and create aStringimmediately, bypassingSimpleInputStreamentirely:The fall-through case (for
TEXT,NTEXT,IMAGE,BINARY,VARBINARY,VECTOR, large values, streaming access, or non-textual target types likegetInt()on a varchar column) continues to use theSimpleInputStreampath.Risk: Low. The fast path produces the exact same
Stringresult as the stream path —DDC.convertStreamToObjectfor non-streaming textual types ultimately doesnew String(stream.getBytes(), charset)and thenconvertStringToObjectreturns the string as-is forCHARACTERcategory JDBC types. The guards (isTextual(), size limit, non-streaming) ensure we only take this path when the result is provably identical.Benchmark Context
ServerDTVImplallocsTDSReaderMarkallocsSimpleInputStreamallocs for stringsclear()callsWorkload:
SELECT * FROM PERF_TABLE— wide table with many columns of mixed types (char,varchar,decimal,float,datetime), ~50% null groups.What is NOT changed
checkClosed()validation chain (safety-critical)PLPInputStream(MAX types) paths