PERF: Optimize fetch API performance - #558
Draft
Jahnvi Thakkar (jahnvi480) wants to merge 2 commits into
Draft
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
… on SUCCESS, __slots__ Row, and C++ Row construction - Cache decoding encoding strings in cursor __init__ to avoid 2 method calls + 2 dict.get() per fetch - Skip DDBCSQLGetAllDiagRecords on SQL_SUCCESS (ODBC spec: zero records on SUCCESS) - Replace param.encode('ascii') try/except with str.isascii() (C-level check) - Class-level _SQL_TO_C_TYPE lookup table (built once, shared across cursors) - Add __slots__ to Row class (eliminates per-instance __dict__, ~232 bytes/row savings) - Add Row._fast_create static method (bypasses __init__ for common case) - Add C++ construct_rows function (builds Row objects in tight C loop, avoiding Python loop overhead) - Zero-copy Row fast path when no converters/UUID processing needed Benchmark results (5-run average, richbench repeat=5 number=5): - Fetch one: -1.7x -> -1.4x (18% improvement) - Fetch many: -1.7x -> -1.3x (24% improvement) - 100 inserts: 4.9x -> 5.6x (14% faster) - SELECT: -1.1x -> -1.0x (on par with pyodbc) Profiler wall clock (50K rows): - fetchall: 176.7ms -> 158.1ms (11% faster) - fetchmany: 166.6ms -> 138.6ms (17% faster) No overlap with PR #549 (execute fast path) or PR #526 (simdutf).
Copy link
Copy Markdown
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 5888-5898 5888 // Set __slots__ via GenericSetAttr (uses descriptor offsets — fast path)
5889 if (PyObject_GenericSetAttr(row, attr_values, row_data) < 0 ||
5890 PyObject_GenericSetAttr(row, attr_column_map, column_map.ptr()) < 0 ||
5891 PyObject_GenericSetAttr(row, attr_cursor, cursor_obj.ptr()) < 0) {
! 5892 Py_DECREF(row);
! 5893 throw py::error_already_set();
! 5894 }
5895
5896 // PyList_SET_ITEM steals the reference — don't Py_DECREF row
5897 PyList_SET_ITEM(result.ptr(), i, row);
5898 }mssql_python/row.pyLines 59-67 59 """
60 # Fast path: no converters and no UUID stringification (common case).
61 # Avoids the converter_map iteration and list copy entirely.
62 if not converter_map and not uuid_str_indices:
! 63 if (
64 cursor
65 and hasattr(cursor.connection, "_output_converters")
66 and cursor.connection._output_converters
67 ):Lines 65-76 65 and hasattr(cursor.connection, "_output_converters")
66 and cursor.connection._output_converters
67 ):
68 # Fallback to original method for backward compatibility
! 69 self._values = self._apply_output_converters(values, cursor)
70 else:
71 # Zero-copy: just store the reference directly
! 72 self._values = values
73 else:
74 # Apply output converters if available using pre-computed converter map
75 if converter_map:
76 self._values = self._apply_output_converters_optimized(values, converter_map)📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 67.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.row.py: 72.3%
mssql_python.pybind.ddbc_bindings.cpp: 74.7%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.3%🔗 Quick Links
|
Sorry, something went wrong.
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.
Work Item / Issue Reference
Summary
This pull request introduces significant performance optimizations to the mssql_python driver's row fetching and construction logic, particularly for the common case where no output converters or UUID processing are required. The changes focus on reducing Python overhead by caching encoding settings, using fast paths for row construction, and moving bulk row creation to C++ for improved speed. Additionally, some code refactoring and simplification have been applied.
Key improvements and optimizations:
Row Fetching and Construction Performance:
Encoding and Decoding Optimization:
Internal Logic Improvements:
These changes collectively reduce per-row overhead, improve memory usage, and make row fetching significantly faster for the most common query scenarios.
Benchmark Results (5-run average, richbench repeat=5 number=5)
Tested back-to-back on the same machine, both branches freshly built from source:
Profiler Wall Clock (50K rows, single run)
Profiler Breakdown: row_wrap phase (50K rows)