Skip to content

Skip Convert.ChangeType on the same-type object read path#338

Merged
Giorgi merged 1 commit into
Giorgi:developfrom
skuirrels:perf/reader-getvalue-fastpath
Jul 11, 2026
Merged

Skip Convert.ChangeType on the same-type object read path#338
Giorgi merged 1 commit into
Giorgi:developfrom
skuirrels:perf/reader-getvalue-fastpath

Conversation

@skuirrels

Copy link
Copy Markdown
Contributor

What's Changed

NumericVectorDataReader.GetValue(offset, targetType) backs the non-generic read
surface — GetValue(ordinal), this[ordinal], GetValues(object[]). It always
ran the boxed value through Convert.ChangeType, even when targetType already
matched the column's natural type. That is the common case, because those APIs
read using the column's ClrType.

Convert.ChangeType does not early-out on same-type for IConvertible
values: it dispatches through ToXxx() and re-boxes the result, so every value
was boxed twice. This adds a value.GetType() == targetType fast path that
returns the already-boxed value directly.

// after building the boxed `value` from the column
if (value.GetType() == targetType)
{
    return value;
}

if (targetType.IsNumeric())
{
    // Convert.ChangeType (unchanged) …
}

Benchmark

Reading 1,000,000 rows × 3 numeric columns (INTEGER, BIGINT, DOUBLE) via
GetValue(ordinal). BenchmarkDotNet, .NET 10, Apple M4 Pro:

GetValue path Before After Change
Mean time 54.50 ms 22.73 ms −58%
Allocated 137.3 MB 68.7 MB −50%

The typed GetFieldValue<T> path is unaffected (~16 ms baseline). The remaining
~69 MB is the unavoidable single box of the object-returning API.

Correctness

Behavior is unchanged: for same-type, ChangeType returned the same value
anyway, and BigInteger (which is not IConvertible) already hit this exact
early-out inside ChangeType. Full existing suite passes locally (6,895 tests).


Third in a small series of allocation-focused changes (#336, #337). Source-only;
the benchmark harness is the DuckDB.NET.Benchmarks project from #336.

NumericVectorDataReader.GetValue(offset, targetType) backs the non-generic read
surface (GetValue(ordinal), this[ordinal], GetValues). It always ran the boxed
value through Convert.ChangeType, even when targetType already matched the
column's natural type — the common case, since those APIs read using the
column's ClrType.

Convert.ChangeType does not early-out on same-type for IConvertible values: it
dispatches through ToXxx() and re-boxes the result, so every value was boxed
twice. Add a value.GetType() == targetType fast path that returns the already
boxed value directly.

Benchmark (read 1,000,000 rows x 3 numeric columns via GetValue, .NET 10, M4 Pro):

  Before: 54.50 ms, 137.3 MB allocated
  After:  22.73 ms,  68.7 MB allocated

-58% time and -50% allocations on the object-returning path (the remaining
allocation is the unavoidable single box of the object API). The typed
GetFieldValue<T> path is unaffected. Behavior is unchanged: ChangeType returned
the same value for same-type, and BigInteger (not IConvertible) already used
this same early-out inside ChangeType.
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.59%. Comparing base (6f5a68d) to head (b208fce).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #338   +/-   ##
========================================
  Coverage    87.58%   87.59%           
========================================
  Files           77       77           
  Lines         3134     3136    +2     
  Branches       463      464    +1     
========================================
+ Hits          2745     2747    +2     
  Misses         275      275           
  Partials       114      114           

☔ View full report in Codecov by Harness.
📢 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.

@skuirrels

Copy link
Copy Markdown
Contributor Author

Just flagging that the macOS red X isn't from this change — it's the Coveralls upload step, which dies trying to brew install its reporter ("Refusing to load formula ... from untrusted tap"). Homebrew changed how it trusts taps and the coveralls action hasn't caught up, so it's failing on macOS for any PR right now, not just this one. The build and tests themselves pass fine on macOS, and Ubuntu/Windows are both green.

Happy to send a small CI fix for it separately if that's helpful.

@Giorgi Giorgi merged commit 1ae01e9 into Giorgi:develop Jul 11, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants