Skip to content

[WIP] Reduce per-column overhead for ResultSet bulk reads - #2934

Draft
machavan wants to merge 2 commits into
mainfrom
dev/machavan/perffixes
Draft

[WIP] Reduce per-column overhead for ResultSet bulk reads#2934
machavan wants to merge 2 commits into
mainfrom
dev/machavan/perffixes

Conversation

@machavan

Copy link
Copy Markdown
Contributor

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 ServerDTVImpl instances (dtv.java)

Problem: Every column on every row allocates a new ServerDTVImpl() when its value is first accessed (via DTV.skipValue(), DTV.getValue(), or DTV.initFromCompressedNull()). When the row is discarded, DTV.clear() sets impl = 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 outgoing ServerDTVImpl in a cachedServerImpl field instead of discarding it. The new helper getOrCreateServerDTVImpl() returns the cached instance (after calling reinit() 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.

Before:  clear() → impl = null   |  skipValue() → impl = new ServerDTVImpl()
After:   clear() → cachedServerImpl = impl; impl = null
         skipValue() → impl = cachedServerImpl.reinit()  (or new if first time)

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. AppDTVImpl instances (from updatable result sets) are not cached — only ServerDTVImpl.


B. Reuse TDSReaderMark objects (dtv.java, IOBuffer.java)

Problem: ServerDTVImpl.getValuePrep() calls tdsReader.mark() for every column value it encounters, which allocates new 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:

  • TDSReaderMark fields changed from final to mutable, and a no-arg constructor + set(packet, offset) method added.
  • TDSReader.markInto(TDSReaderMark) added alongside the existing mark() — writes into an existing mark object instead of creating a new one.
  • ServerDTVImpl now pre-allocates one TDSReaderMark at construction time and reuses it via markInto(). A hasValueMark boolean replaces the valueMark == null checks.
  • The existing mark() method is unchanged and still used by all other callers (scroll windows, fetch buffer reset, LOB streams, etc.).
Before:  valueMark = tdsReader.mark()  → new TDSReaderMark(packet, offset)
After:   tdsReader.markInto(valueMark)  → valueMark.set(packet, offset)

Risk: Low. The mutable TDSReaderMark is private to ServerDTVImpl and never exposed externally. All other mark() callsites are untouched. The setPositionAfterStreamed() path (adaptive streaming) continues to use the allocating mark().


C. Fast-path readUnsignedByte() (IOBuffer.java)

Problem: TDSReader.readUnsignedByte() unconditionally calls ensurePayload() (a method call) before reading a single byte. This method is called as the length-prefix reader for every BYTELENTYPE column (e.g., char(1), varchar(64), nullable int/float/decimal). In the vast majority of cases the current packet still has data — the ensurePayload() call is a no-op.

Fix: Add an inline fast-path check payloadOffset < currentPacket.payloadLength before the ensurePayload() call, matching the pattern already used by readInt(), readShort(), and readLong():

// Before
final int readUnsignedByte() throws SQLServerException {
    if (!ensurePayload())
        throwInvalidTDS();
    return currentPacket.payload[payloadOffset++] & 0xFF;
}

// After
final int readUnsignedByte() throws SQLServerException {
    if (payloadOffset < currentPacket.payloadLength) {
        return currentPacket.payload[payloadOffset++] & 0xFF;
    }
    if (!ensurePayload())
        throwInvalidTDS();
    return currentPacket.payload[payloadOffset++] & 0xFF;
}

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/exiting in hot-path getters (SQLServerResultSet.java)

Problem: Every ResultSet getter (getInt, getLong, getString, getDouble, getBigDecimal, getTimestamp, wasNull) unconditionally calls loggerExternal.entering(getClassNameLogging(), ...) and loggerExternal.exiting(getClassNameLogging(), ...). Even when logging is disabled (the normal case), each call involves:

  • A getClassNameLogging() method call
  • Autoboxing of primitive arguments (e.g., int columnIndexInteger)
  • Entry into java.util.logging.Logger.entering() which internally calls isLoggable(Level.FINER)

For wide result sets this adds up to billions of no-op logging calls.

Fix: Wrap entering/exiting calls in if (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)).

// Before
public int getInt(int columnIndex) throws SQLServerException {
    loggerExternal.entering(getClassNameLogging(), "getInt", columnIndex);
    checkClosed();
    Integer value = (Integer) getValue(columnIndex, JDBCType.INTEGER);
    loggerExternal.exiting(getClassNameLogging(), "getInt", value);
    return null != value ? value : 0;
}

// After
public int getInt(int columnIndex) throws SQLServerException {
    if (loggerExternal.isLoggable(java.util.logging.Level.FINER))
        loggerExternal.entering(getClassNameLogging(), "getInt", columnIndex);
    checkClosed();
    Integer value = (Integer) getValue(columnIndex, JDBCType.INTEGER);
    if (loggerExternal.isLoggable(java.util.logging.Level.FINER))
        loggerExternal.exiting(getClassNameLogging(), "getInt", value);
    return null != value ? value : 0;
}

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/exiting performs 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 calling column.clear() on each one individually. For a wide result set where all columns were read, this is hundreds of clear() 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 trigger ServerDTVImpl caching (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 a CHAR, VARCHAR, NCHAR, or NVARCHAR column, ServerDTVImpl.getValue() creates a new SimpleInputStream(tdsReader, valueLength, ...), which is then passed to DDC.convertStreamToObject(). That method calls stream.getBytes() (allocates byte[valueLength], reads, closes the stream) → new String(bytes, charset). The SimpleInputStream creation involves:

  • Object allocation + BaseInputStream constructor (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 SimpleInputStream allocations + millions of byte[] allocations where the stream is immediately consumed and discarded.

Fix: For CHAR/VARCHAR/NCHAR/NVARCHAR columns 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 TDSReader into a byte[] and create a String immediately, bypassing SimpleInputStream entirely:

// Fast path
byte[] bytes = new byte[valueLength];
tdsReader.readBytes(bytes, 0, valueLength);
convertedValue = new String(bytes, typeInfo.getCharset());

The fall-through case (for TEXT, NTEXT, IMAGE, BINARY, VARBINARY, VECTOR, large values, streaming access, or non-textual target types like getInt() on a varchar column) continues to use the SimpleInputStream path.

Risk: Low. The fast path produces the exact same String result as the stream path — DDC.convertStreamToObject for non-streaming textual types ultimately does new String(stream.getBytes(), charset) and then convertStringToObject returns the string as-is for CHARACTER category JDBC types. The guards (isTextual(), size limit, non-streaming) ensure we only take this path when the result is provably identical.


Benchmark Context

Metric Before After (est.)
ServerDTVImpl allocs millions ~1 per column (reused)
TDSReaderMark allocs millions ~1 per column (reused)
SimpleInputStream allocs for strings millions ~0 (fast path)
No-op logging method calls billions ~0 (guarded)
Per-column clear() calls millions millions (same, but enables caching)

Workload: 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)
  • Scrollable / updatable / server-cursor ResultSet paths
  • LOB streaming, adaptive response buffering
  • Always Encrypted decryption paths
  • PLPInputStream (MAX types) paths
  • Any public API signatures or behavior

- 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
@github-project-automation github-project-automation Bot moved this to In progress in MSSQL JDBC Apr 11, 2026
@machavan machavan changed the title Reduce per-column overhead for ResultSet bulk reads [WIP] Reduce per-column overhead for ResultSet bulk reads Apr 11, 2026
@codecov

codecov Bot commented Apr 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.67677% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.44%. Comparing base (e39559a) to head (0bf95c3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 51.85% 0 Missing and 26 partials ⚠️
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 81.81% 0 Missing and 6 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

1 participant