PERF: Reduce initial buffer allocation for SQLColumns fetchall - #824
Jahnvi Thakkar (jahnvi480) wants to merge 2 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR Performance Report
|
| Database task | Unix / SQL Server 2022 | Unix / SQL Server 2025 |
|---|---|---|
| Catalog columns / 118 rows | 33.3% slower | 32.9% slower |
The largest recorded phase increases for these tasks are shown below. Phase timings are supporting evidence, not root-cause proof.
Coverage: 2 of 2 environments completed. Advisory result; does not block merging.
Measured timings
| Environment | Database task | Before | After | Change |
|---|---|---|---|---|
| Unix / SQL Server 2022 | Catalog columns / 118 rows | 7.414 ms | 9.670 ms | +33.3% |
| Unix / SQL Server 2025 | Catalog columns / 118 rows | 8.651 ms | 11.871 ms | +32.9% |
Performance diagnostics
Phase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed.
Unix / SQL Server 2022
Catalog columns / 118 rows: py::fetchall::cpp_call +2.296 ms; ddbc::FetchAll_wrap +2.296 ms; ddbc::SQLBindColums +2.270 ms. Call changes: ddbc::FetchBatchData (2 -> 4 calls); ddbc::FetchBatchData::SQLFetchScroll_call (2 -> 4 calls); ddbc::FetchBatchData::cache_column_metadata (1 -> 3 calls).
Catalog columns / 2,111 rows: ddbc::SQLColumns_wrap +1.110 ms; ddbc::SQLBindColums +0.137 ms; ddbc::FetchBatchData::construct_rows +0.024 ms. Call changes: ddbc::FetchBatchData (4 -> 6 calls); ddbc::FetchBatchData::SQLFetchScroll_call (4 -> 6 calls); ddbc::FetchBatchData::cache_column_metadata (3 -> 5 calls).
Unix / SQL Server 2025
Catalog columns / 118 rows: py::fetchall::cpp_call +3.168 ms; ddbc::FetchAll_wrap +3.164 ms; ddbc::SQLBindColums +3.094 ms. Call changes: ddbc::FetchBatchData (2 -> 4 calls); ddbc::FetchBatchData::SQLFetchScroll_call (2 -> 4 calls); ddbc::FetchBatchData::cache_column_metadata (1 -> 3 calls).
Catalog columns / 2,111 rows: ddbc::SQLColumns_wrap +0.723 ms; ddbc::SQLBindColums +0.131 ms; ddbc::FetchBatchData::cache_column_metadata +0.018 ms. Call changes: ddbc::FetchBatchData (4 -> 6 calls); ddbc::FetchBatchData::SQLFetchScroll_call (4 -> 6 calls); ddbc::FetchBatchData::cache_column_metadata (3 -> 5 calls).
All database tasks and timings
Unix / SQL Server 2022
| Database task | Before | After | Paired change | Result |
|---|---|---|---|---|
| Connection opening | 10.437 ms | 10.147 ms | -4.0% | no signal |
| SELECT queries | 1.067 ms | 1.050 ms | -2.3% | no signal |
| Row insertion | 34.238 ms | 33.710 ms | -1.3% | no signal |
| Executemany inserts | 153.375 ms | 153.276 ms | +0.6% | no signal |
| Fetch-all queries | 117.647 ms | 117.844 ms | +0.6% | no signal |
| Row-by-row fetching | 14.033 ms | 13.909 ms | -0.9% | no signal |
| Batched row fetching | 115.749 ms | 115.131 ms | -0.9% | no signal |
| Transaction commit and rollback | 110.982 ms | 110.993 ms | -0.5% | no signal |
| Arrow row fetching | 91.785 ms | 93.867 ms | +3.2% | no signal |
| 100,000-row insertion | 434.927 ms | 450.953 ms | +2.5% | no signal |
| Row fetching in batches of 100 | 119.727 ms | 120.643 ms | +0.7% | no signal |
| Row fetching in batches of 10,000 | 132.006 ms | 118.958 ms | -9.9% | no signal |
| Repeated positional queries | 32.918 ms | 33.213 ms | -0.4% | no signal |
| Repeated named-parameter queries | 34.903 ms | 34.872 ms | -0.3% | no signal |
| Legacy 100,000-row insertion | 353.745 ms | 358.972 ms | -1.1% | no signal |
| Insertion with explicit input sizes | 476.660 ms | 477.047 ms | -0.6% | no signal |
| Joined aggregation queries | 176.284 ms | 177.981 ms | +0.4% | no signal |
| Large joined-result fetching | 168.192 ms | 172.161 ms | +2.4% | no signal |
| 1.2-million-row fetching | 3440.627 ms | 3447.864 ms | +1.6% | no signal |
| Common table expression queries | 5.274 ms | 5.300 ms | +0.7% | no signal |
| 256 KiB VARCHAR(MAX) / fetchall() | 1.295 ms | 1.320 ms | +2.4% | no signal |
| Catalog columns / 2 rows | 64.286 ms | 55.543 ms | -7.3% | no signal |
| Catalog columns / 118 rows | 7.414 ms | 9.670 ms | +33.3% | consistent slowdown |
| Catalog columns / 2,111 rows | 140.422 ms | 141.329 ms | -0.3% | no signal |
Unix / SQL Server 2025
| Database task | Before | After | Paired change | Result |
|---|---|---|---|---|
| Connection opening | 96.547 ms | 96.694 ms | +0.3% | no signal |
| SELECT queries | 1.091 ms | 1.080 ms | -0.4% | no signal |
| Row insertion | 34.409 ms | 35.589 ms | +4.5% | no signal |
| Executemany inserts | 150.157 ms | 150.127 ms | +0.9% | no signal |
| Fetch-all queries | 121.048 ms | 120.224 ms | +0.7% | no signal |
| Row-by-row fetching | 14.307 ms | 14.315 ms | +0.5% | no signal |
| Batched row fetching | 116.592 ms | 115.646 ms | -0.6% | no signal |
| Transaction commit and rollback | 114.671 ms | 115.011 ms | -0.1% | no signal |
| Arrow row fetching | 92.193 ms | 94.277 ms | +1.4% | no signal |
| 100,000-row insertion | 427.191 ms | 455.041 ms | +8.7% | no signal |
| Row fetching in batches of 100 | 121.894 ms | 123.572 ms | +1.4% | no signal |
| Row fetching in batches of 10,000 | 127.620 ms | 125.544 ms | +1.5% | no signal |
| Repeated positional queries | 34.085 ms | 33.646 ms | -0.4% | no signal |
| Repeated named-parameter queries | 36.376 ms | 35.979 ms | -0.8% | no signal |
| Legacy 100,000-row insertion | 363.475 ms | 346.309 ms | -1.2% | no signal |
| Insertion with explicit input sizes | 480.979 ms | 514.931 ms | +4.9% | no signal |
| Joined aggregation queries | 158.863 ms | 160.644 ms | +0.8% | no signal |
| Large joined-result fetching | 175.799 ms | 183.879 ms | +1.8% | no signal |
| 1.2-million-row fetching | 3517.967 ms | 3511.762 ms | +0.1% | no signal |
| Common table expression queries | 5.199 ms | 5.077 ms | -0.6% | no signal |
| 256 KiB VARCHAR(MAX) / fetchall() | 1.521 ms | 1.535 ms | +3.3% | no signal |
| Catalog columns / 2 rows | 83.562 ms | 83.005 ms | +4.9% | no signal |
| Catalog columns / 118 rows | 8.651 ms | 11.871 ms | +32.9% | consistent slowdown |
| Catalog columns / 2,111 rows | 170.144 ms | 170.569 ms | -1.7% | no signal |
Build and measurement details
PR head: a28f9db65112c558206422bcc198e2cc30ace580
Base: fead15c30e49172bab643bc9cc5504936e86459e
Measured merge: 61e1a8da7b433fe8eaabf27e285d42dbb17f3c18
- Unix / SQL Server 2022: Python 3.12.3, x86_64, SQL 16.0.4295.3; 5 paired comparisons and 1 warmup.
- Unix / SQL Server 2025: Python 3.12.3, x86_64, SQL 17.0.5005.3; 5 paired comparisons and 1 warmup.
A consistent change requires more than 20% median paired movement, at least 1 ms between the median runtimes, and at least 80% of pairs exceeding the relative threshold in the same direction. A slowdown without enough pair agreement is reported as inconsistent.
The displayed change is the median of paired before-and-after ratios. It is not recalculated from the two displayed median runtimes.
Both revisions use profiling-enabled builds on the same agent and database, with alternating order and discarded warmups. Results are diagnostic and do not represent production-wheel latency.
Raw samples and logs are attached to the ADO run as profiler-* artifacts.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The tests do not assert the core allocation-tier optimization, allowing silent performance regression.
Review effort: Balanced
Findings: 1
What changed in this PR
Reduces initial native buffer allocation for Cursor.columns().fetchall() while preserving fetch behavior.
Changes:
- Adds SQLColumns-specific metadata state.
- Grows fetch buffers from 10 to existing capacity tiers.
- Expands catalog-fetch parity and cursor-state tests.
| File | Description |
|---|---|
mssql_python/pybind/result_metadata.hpp |
Tracks the SQLColumns result hint. |
mssql_python/pybind/ddbc_bindings.cpp |
Implements adaptive buffer growth. |
tests/test_004_cursor.py |
Expands catalog fetch coverage. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (metadataSnapshot.catalogResult) { | ||
| fetchSize = std::min(fetchSize, 10); |
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 6264-6273 6264 CheckFetchError(StatementHandle, ret);
6265 if (!SQL_SUCCEEDED(ret) && ret != SQL_NO_DATA) {
6266 LOG("FetchAll_wrap: Error when fetching data - SQLRETURN=%d", ret);
6267 return ret;
! 6268 }
! 6269 if (SQL_SUCCEEDED(ret) && numRowsFetched == static_cast<SQLULEN>(fetchSize) &&
6270 fetchSize < maxFetchSize) {
6271 break;
6272 }
6273 }📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 62.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 79.3%
mssql_python.pybind.connection.connection_pool.cpp: 82.3%
mssql_python.pybind.connection.connection.cpp: 83.1%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.pybind.fetch_temporal.hpp: 92.1%🔗 Quick Links
|
Add catalog workloads for 2, 118, and 2111 rows with untimed fixture setup, raw metadata and typed row validation, and rollback cleanup. Wire the workloads into the existing advisory report and cover their contracts with driver-free tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| // Unbind while these buffers are still alive, before allocating the next tier. | ||
| fetchStateGuard.close(); |
| assert cpp["ddbc::SQLColumns_wrap"]["calls"] == 1 | ||
| assert cpp["ddbc::FetchAll_wrap"]["calls"] == 1 |
Work Item / Issue Reference
Summary
Reduce initial native bound-buffer allocation when
fetchall()reads results fromCursor.columns(), without changing the public API, metadata values, ordering, types, description labels, orarraysize.SQLColumnsresult generations through the existing metadata state, clearing the hint through existing reset/failure paths.min(original capacity, 10)and grow through 100/1000 after full batches, up to the original size-derived maximum. Continue to real EOF and unbind before replacing storage without closing the active result. Ordinary-result capacity selection and LOB fallback remain unchanged.catalog_columns_2,catalog_columns_118, andcatalog_columns_2111to the existing PR Performance Report, with benchmark/report contract coverage. Each timescolumns()+fetchall()on a fresh cursor; fixture DDL, complete raw-description/typed-row validation against a subsequent rowwise drain, and rollback cleanup remain outside the timed window.Validation and performance evidence
The unchanged native candidate was compared with
fead15c30e49172bab643bc9cc5504936e86459eon a shared Linux x64 host using Release builds with profiling compiled out.The September 29 run's overall status is FAIL: its post-stop host-preservation check timed out after 15 seconds. Preservation is unproven, not disproved. The following timings are therefore descriptive, not a clean overall qualification pass.
Lower ratios are faster. The 118-row median was 5.23% worse and order-dependent: base-first ratio 1.1620 versus candidate-first 0.9689. No conclusive attribution or universal improvement/non-regression claim is made. The two-row fixtures differ between dates and are not the same measurement identity.
New CI benchmark coverage
The new catalog scenarios use profiling-enabled builds and distinct UUID-prefixed fixtures: 2, 118, or 2,111 nullable INT columns across one, one, or three tables. They validate all 29 provider fields and verify one native SQLColumns and one FetchAll call in the measured window. They do not measure allocation bytes or establish Release-OFF latency, and the historical ratios above are not results for these new fixtures. Advisory reporting thresholds are unchanged.
Local validation: 19 focused catalog/registry cases passed. The full
test_036_profiler_ci.pyattempt had 140 passed, 1 skipped, and 2 failures: two existing legacy-insert cases attempted a driver import but the compiled extension was unavailable. No native binary loaded and no SQL ran. The explicitly driver-free selection excluding those two cases passed with 140 passed, 1 skipped, and 2 deselected. Formatting and diff checks passed. Live execution of the new catalog benchmarks is left to automatic PR CI; no new live benchmark result is claimed here.Tradeoffs and remaining qualification
Growth can add two bind/unbind phases; exact full-tier boundaries can allocate the next tier only to discover EOF. Existing cleanup-failure ownership behavior is reused, not repaired or certified by this change. Allocation-byte measurements, native fault injection, and Windows/macOS/ARM validation remain pending. Diagnostic overlays and scratch harnesses are excluded. These are selected-test results, not full-suite or cross-platform validation.