Repository navigation
PERF: Speed up row-to-dictionary conversion and add CI coverage - #847
Merged
Merged
Conversation
Share lazy result-shape metadata and add guarded native mapping iteration and lookup while preserving live-view behavior. Add one warm row-to-dictionary workload to the existing performance CI registry with focused contract coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove overlapping matrices while retaining native ownership, invalidation, dispatch, custom behavior and mapping semantics. Extend existing controls only for measured assertion gaps; production and the row_to_dict workload stay unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR Performance Report✅ Performance improved1 database task consistently improved across 2 measured environments. No consistent slowdowns were detected. 1 IMPROVEMENT 0 SLOWDOWNS 2/2 ENVIRONMENTS Signal fingerprint
Coverage: 2 of 2 environments completed. Advisory result; does not block merging. Measured timings
Performance diagnosticsPhase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed. Unix / SQL Server 202210,000 rows x 32 columns / dict(row._mapping) (warm): no measured phase delta. Unix / SQL Server 202510,000 rows x 32 columns / dict(row._mapping) (warm): no measured phase delta. All database tasks and timingsUnix / SQL Server 2022
Unix / SQL Server 2025
Build and measurement detailsPR head:
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 |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Native ownership and invalidation changes require human review, while cross-platform validation and broader performance qualification remain incomplete.
0 open findings
What changed in this PR
Optimizes dict(row._mapping) through shared lazy metadata and guarded native access, with focused compatibility tests and performance-CI coverage.
Changes:
- Adds native mapping iteration, lookup, and cache invalidation.
- Shares column metadata across synchronous and asynchronous cursor rows.
- Adds one warm-conversion benchmark, expanding the registry to 23 workloads.
| File | Description |
|---|---|
| tests/test_row_mapping_metadata.py | Tests mapping semantics, mutation guards, and lifetimes. |
| tests/test_036_profiler_ci.py | Tests benchmark timing boundaries and diagnostics validation. |
| mssql_python/row.py | Adds shared metadata and native-backed mapping views. |
| mssql_python/pybind/row_mapping.hpp | Implements guarded native mapping access. |
| mssql_python/pybind/row_mapping_cache.hpp | Adds watched schemas and ownership handling. |
| mssql_python/pybind/ddbc_bindings.cpp | Exposes the native mapping factory. |
| mssql_python/cursor.py | Shares lazy column metadata across fetched rows. |
| mssql_python/async_query/async_cursor.py | Uses shared metadata for asynchronous results. |
| eng/profiler_benchmarks/workloads.py | Adds the warm row-to-dictionary workload. |
| eng/profiler_benchmarks/report.py | Validates and reports the new workload. |
| eng/profiler_benchmarks/README.md | Documents timing boundaries and qualification limits. |
| eng/profiler_benchmarks/controller.py | Allows empty native diagnostics only for the new workload. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/row_mapping.hppLines 140-148 140 PyMemberDef* member = reinterpret_cast<PyMemberDescrObject*>(descriptor)->d_member;
141 if (member->type != T_OBJECT_EX || member->offset < static_cast<Py_ssize_t>(sizeof(PyObject)) ||
142 member->offset > type->tp_basicsize - static_cast<Py_ssize_t>(sizeof(PyObject*)) ||
143 member->offset % static_cast<Py_ssize_t>(alignof(PyObject*)) != 0)
! 144 return -1;
145 return member->offset;
146 }
147
148 inline bool valid_iterator_type(PyObject* defaults) {Lines 163-178 163 PyErr_SetString(PyExc_TypeError, "Invalid RowMapping configuration");
164 return false;
165 }
166 if (!valid_iterator_type(defaults))
! 167 return false;
168 unsigned long version = PyLong_AsUnsignedLong(PyTuple_GET_ITEM(defaults, ROW_VERSION));
169 if (version == static_cast<unsigned long>(-1) && PyErr_Occurred())
! 170 return false;
171 unsigned long names_version = PyLong_AsUnsignedLong(
172 PyTuple_GET_ITEM(defaults, NAMES_VERSION));
173 if (names_version == static_cast<unsigned long>(-1) && PyErr_Occurred())
! 174 return false;
175 RowMappingCache::Watcher* watcher = nullptr;
176 PyObject* owner = PyTuple_GET_ITEM(defaults, WATCHER_OWNER);
177 if (owner != Py_None) {
178 watcher = static_cast<RowMappingCache::Watcher*>(Lines 177-191 177 if (owner != Py_None) {
178 watcher = static_cast<RowMappingCache::Watcher*>(
179 PyCapsule_GetPointer(owner, "mssql.RowMappingWatcher"));
180 if (!watcher)
! 181 return false;
182 }
183 PyObject* members = PyTuple_GET_ITEM(defaults, ROW_MEMBERS);
184 if (!PyTuple_CheckExact(members) || PyTuple_GET_SIZE(members) != ROW_FIELD_COUNT) {
! 185 PyErr_SetString(PyExc_TypeError, "Invalid RowMapping member configuration");
! 186 return false;
! 187 }
188 access.row_version = static_cast<unsigned int>(version);
189 access.names_version = static_cast<unsigned int>(names_version);
190 access.watcher = watcher;
191 auto* row_type = reinterpret_cast<PyTypeObject*>(PyTuple_GET_ITEM(defaults, ROW_TYPE));Lines 234-250 234 Py_TPFLAGS_DISALLOW_INSTANTIATION, slots,
235 };
236 py::object type = steal(PyType_FromSpec(&spec));
237 if (!type)
! 238 throw py::error_already_set();
239 auto* native_type = reinterpret_cast<PyTypeObject*>(type.ptr());
240 py::object owner = steal(native_type->tp_alloc(native_type, 0));
241 if (!owner)
! 242 throw py::error_already_set();
243 auto* configuration = reinterpret_cast<Configuration*>(owner.ptr());
244 configuration->defaults = Py_NewRef(defaults.ptr());
245 if (!read_configuration(defaults.ptr(), configuration->access))
! 246 throw py::error_already_set();
247 return owner;
248 }
249
250 inline PyObject* create(PyTypeObject* type, PyObject*, PyObject*) {Lines 250-258 250 inline PyObject* create(PyTypeObject* type, PyObject*, PyObject*) {
251 py::object configuration = steal(PyObject_GetAttrString(
252 reinterpret_cast<PyObject*>(type), "_row_mapping_defaults"));
253 if (!configuration)
! 254 return nullptr;
255 py::object defaults;
256 AccessConfiguration access{};
257 if (is_configuration(configuration.ptr())) {
258 auto* native = reinterpret_cast<Configuration*>(configuration.ptr());Lines 256-266 256 AccessConfiguration access{};
257 if (is_configuration(configuration.ptr())) {
258 auto* native = reinterpret_cast<Configuration*>(configuration.ptr());
259 if (!native->defaults) {
! 260 PyErr_SetString(PyExc_TypeError, "Invalid RowMapping configuration");
! 261 return nullptr;
! 262 }
263 defaults = borrow(native->defaults);
264 access = native->access;
265 } else {
266 defaults = std::move(configuration);Lines 271-279 271 if (!valid_iterator_type(defaults.ptr()))
272 return nullptr;
273 auto* self = reinterpret_cast<View*>(type->tp_alloc(type, 0));
274 if (!self)
! 275 return nullptr;
276 self->defaults = defaults.release().ptr();
277 self->configuration = configuration.release().ptr();
278 self->row_version = access.row_version;
279 self->names_version = access.names_version;Lines 347-355 347 py::object names;
348 if (plain_row) {
349 names = row_field(self, row, NAMES, ATTR_NAMES);
350 if (!names)
! 351 return {};
352 }
353 if (!plain_row || names.is_none()) {
354 names = steal(PyObject_CallNoArgs(hook.ptr()));
355 }Lines 382-390 382 Py_TYPE(names)->tp_version_tag != self->names_version)
383 return 0;
384 if ((!Py_TYPE(row)->tp_dictoffset && !(Py_TYPE(row)->tp_flags & Py_TPFLAGS_MANAGED_DICT)) ||
385 (!Py_TYPE(names)->tp_dictoffset && !(Py_TYPE(names)->tp_flags & Py_TPFLAGS_MANAGED_DICT)))
! 386 return 0;
387 for (int field = 0; field < ROW_FIELD_COUNT; ++field)
388 if (self->row_offsets[field] < 0)
389 return 0;
390 PyObject* values = stored_field(self, row, VALUES);Lines 399-410 399 // Materialize only the ordinary instance dictionary, never values/getset storage.
400 // Its stable pointer also detects wholesale __dict__ replacement without a watcher event.
401 py::object row_dictionary = steal(PyObject_GenericGetDict(row, nullptr));
402 if (!row_dictionary)
! 403 return -1;
404 py::object names_dictionary = steal(PyObject_GenericGetDict(names, nullptr));
405 if (!names_dictionary)
! 406 return -1;
407 if (!builtin_keys(row_dictionary.ptr()) || !builtin_keys(names_dictionary.ptr()))
408 return 0;
409 PyObject** row_pointer = _PyObject_GetDictPtr(row);
410 PyObject** names_pointer = _PyObject_GetDictPtr(names);Lines 408-430 408 return 0;
409 PyObject** row_pointer = _PyObject_GetDictPtr(row);
410 PyObject** names_pointer = _PyObject_GetDictPtr(names);
411 if (!row_pointer || !names_pointer) {
! 412 if (PyErr_Occurred())
! 413 return -1;
! 414 return 0;
! 415 }
416 if (PyDict_Watch(self->watcher->id, columns) < 0 ||
417 PyDict_Watch(self->watcher->id, unique) < 0 ||
418 PyDict_Watch(self->watcher->id, row_dictionary.ptr()) < 0 ||
419 PyDict_Watch(self->watcher->id, names_dictionary.ptr()) < 0)
! 420 return -1;
421 if (!builtin_keys(names_dictionary.ptr()))
! 422 return 0;
423 PyObject* cached = PyDict_GetItemWithError(
424 names_dictionary.ptr(), PyTuple_GET_ITEM(self->defaults, ATTR_PREPARED));
425 if (!cached && PyErr_Occurred())
! 426 return -1;
427 auto* schema_class = reinterpret_cast<PyTypeObject*>(
428 PyTuple_GET_ITEM(self->defaults, SCHEMA_TYPE));
429 py::object schema_owner;
430 Schema* schema = nullptr;Lines 431-448 431 if (cached && Py_IS_TYPE(cached, schema_class)) {
432 schema_owner = borrow(cached);
433 schema = reinterpret_cast<Schema*>(cached);
434 } else if (cached && cached != Py_None) {
! 435 return 0;
! 436 }
437 if (!schema || schema->epoch != self->watcher->epoch ||
438 schema->columns != columns || schema->unique != unique) {
439 py::object indices = steal(PyDict_New());
440 if (!indices)
! 441 return -1;
442 schema_owner = steal(schema_class->tp_alloc(schema_class, 0));
443 if (!schema_owner)
! 444 return -1;
445 schema = reinterpret_cast<Schema*>(schema_owner.ptr());
446 std::uint64_t epoch = self->watcher->epoch;
447 if (!builtin_keys(columns, true) || !builtin_keys(unique))
448 return 0;Lines 446-454 446 std::uint64_t epoch = self->watcher->epoch;
447 if (!builtin_keys(columns, true) || !builtin_keys(unique))
448 return 0;
449 if (allocate_entries(schema, PyDict_Size(unique)) < 0)
! 450 return -1;
451 Py_ssize_t position = 0;
452 PyObject *key, *unused;
453 while (PyDict_Next(unique, &position, &key, &unused)) {
454 PyObject* index = PyDict_GetItemWithError(columns, key);Lines 454-464 454 PyObject* index = PyDict_GetItemWithError(columns, key);
455 if (!index)
456 return PyErr_Occurred() ? -1 : 0;
457 if (PyDict_SetItem(indices.ptr(), key, index) < 0)
! 458 return -1;
459 if (append_entry(schema, key, index) < 0)
! 460 return -1;
461 }
462 schema->columns = Py_NewRef(columns);
463 schema->unique = Py_NewRef(unique);
464 schema->indices = indices.release().ptr();Lines 463-481 463 schema->unique = Py_NewRef(unique);
464 schema->indices = indices.release().ptr();
465 schema->watcher_owner = Py_NewRef(PyTuple_GET_ITEM(self->defaults, WATCHER_OWNER));
466 if (PyDict_Watch(self->watcher->id, schema->indices) < 0)
! 467 return -1;
468 if (self->watcher->epoch != epoch)
! 469 return 0;
470 if (!builtin_keys(names_dictionary.ptr()))
! 471 return 0;
472 // This single watched write advances the epoch. Any additional mutation deopts.
473 if (PyDict_SetItem(names_dictionary.ptr(),
474 PyTuple_GET_ITEM(self->defaults, ATTR_PREPARED), schema_owner.ptr()) < 0)
! 475 return -1;
476 if (!self->watcher->enabled || self->watcher->epoch != epoch + 1)
! 477 return 0;
478 schema->epoch = self->watcher->epoch;
479 ++self->schema_builds;
480 }
481 // Allocations above may run GC/finalizers: seal only after rechecking all dependencies.Lines 484-501 484 Py_TYPE(self)->tp_version_tag != self->view_version ||
485 Py_TYPE(names)->tp_version_tag != self->names_version ||
486 *row_pointer != row_dictionary.ptr() || *names_pointer != names_dictionary.ptr() ||
487 stored_field(self, row, NAMES) != names || stored_field(self, row, COLUMNS) != columns)
! 488 return 0;
489 if (!builtin_keys(row_dictionary.ptr()) || !builtin_keys(names_dictionary.ptr()))
! 490 return 0;
491 if (PyDict_GetItemWithError(row_dictionary.ptr(), PyTuple_GET_ITEM(self->defaults, ATTR_KEYS)))
! 492 return 0;
493 if (PyErr_Occurred())
! 494 return -1;
495 if (PyDict_GetItemWithError(names_dictionary.ptr(),
496 PyTuple_GET_ITEM(self->defaults, ATTR_UNIQUE)) != unique)
! 497 return PyErr_Occurred() ? -1 : 0;
498 self->row_dict_pointer = row_pointer;
499 self->names_dict_pointer = names_pointer;
500 self->prepared_row = row_owner.release().ptr();
501 self->prepared_names = names_owner.release().ptr();Lines 581-591 581 }
582
583 inline PyObject* subscript(View* self, PyObject* key) {
584 if (!self->defaults) {
! 585 PyErr_SetString(PyExc_AttributeError, "RowMapping has no row");
! 586 return nullptr;
! 587 }
588 PyObject* prepared_result;
589 if (prepared_subscript(self, key, &prepared_result))
590 return prepared_result;
591 ++self->slow_lookups;Lines 591-599 591 ++self->slow_lookups;
592 if (!PyUnicode_Check(key)) {
593 int is_string = PyObject_IsInstance(key, reinterpret_cast<PyObject*>(&PyUnicode_Type));
594 if (is_string < 0)
! 595 return nullptr;
596 if (!is_string)
597 return missing_key(key);
598 }
599 py::object row = observed_row(self);Lines 608-616 608 if (PyUnicode_CheckExact(key) &&
609 reinterpret_cast<PyObject*>(Py_TYPE(names.ptr())) == names_type) {
610 py::object unique = get_unique(self, names.ptr());
611 if (!unique)
! 612 return nullptr;
613 contains = PySequence_Contains(unique.ptr() == Py_False ? names.ptr() : unique.ptr(), key);
614 } else {
615 // str subclasses may be unhashable or customize equality: retain tuple lookup.
616 contains = PySequence_Contains(names.ptr(), key);Lines 615-623 615 // str subclasses may be unhashable or customize equality: retain tuple lookup.
616 contains = PySequence_Contains(names.ptr(), key);
617 }
618 if (contains < 0)
! 619 return nullptr;
620 if (!contains)
621 return missing_key(key);
622
623 // A custom key hook can replace the view's row during the membership check.Lines 622-630 622
623 // A custom key hook can replace the view's row during the membership check.
624 py::object current_row = observed_row(self);
625 if (!current_row)
! 626 return nullptr;
627 plain_row = plain_row && current_row.ptr() == row.ptr();
628 row = std::move(current_row);
629 if (plain_row && PyUnicode_CheckExact(key) &&
630 Py_TYPE(row.ptr())->tp_version_tag == self->row_version) {Lines 629-641 629 if (plain_row && PyUnicode_CheckExact(key) &&
630 Py_TYPE(row.ptr())->tp_version_tag == self->row_version) {
631 py::object columns = row_field(self, row.ptr(), COLUMNS, ATTR_COLUMNS);
632 if (!columns)
! 633 return nullptr;
634 if (PyDict_CheckExact(columns.ptr())) {
635 int found = PyDict_Contains(columns.ptr(), key);
636 if (found < 0)
! 637 return nullptr;
638 columns = py::object();
639 if (found) {
640 py::object values = row_field(self, row.ptr(), VALUES, ATTR_VALUES);
641 if (!values)Lines 642-653 642 return nullptr;
643 // Equality on a custom dict key can replace the row's column map.
644 columns = row_field(self, row.ptr(), COLUMNS, ATTR_COLUMNS);
645 if (!columns)
! 646 return nullptr;
647 py::object index = steal(PyObject_GetItem(columns.ptr(), key));
648 if (!index)
! 649 return nullptr;
650 return PyObject_GetItem(values.ptr(), index.ptr());
651 }
652
653 py::object lower = steal(PyObject_GetAttrString(row.ptr(), "_column_map_lower"));Lines 651-679 651 }
652
653 py::object lower = steal(PyObject_GetAttrString(row.ptr(), "_column_map_lower"));
654 if (!lower)
! 655 return nullptr;
656 if (!lower.is_none()) {
! 657 lower = steal(PyObject_GetAttrString(row.ptr(), "_column_map_lower"));
! 658 if (!lower)
! 659 return nullptr;
! 660 py::object get = steal(PyObject_GetAttrString(lower.ptr(), "get"));
! 661 if (!get)
! 662 return nullptr;
! 663 py::object lowered = steal(PyObject_CallMethod(key, "lower", nullptr));
! 664 if (!lowered)
! 665 return nullptr;
! 666 py::object index = steal(PyObject_CallOneArg(get.ptr(), lowered.ptr()));
! 667 if (!index)
! 668 return nullptr;
! 669 if (!index.is_none()) {
! 670 py::object values = row_field(self, row.ptr(), VALUES, ATTR_VALUES);
! 671 if (!values)
! 672 return nullptr;
! 673 return PyObject_GetItem(values.ptr(), index.ptr());
! 674 }
! 675 }
676 py::object message = steal(PyUnicode_FromFormat("Row has no column '%U'", key));
677 if (message)
678 PyErr_SetObject(PyExc_KeyError, message.ptr());
679 return nullptr;Lines 733-749 733 if (reinterpret_cast<PyObject*>(Py_TYPE(names.ptr())) ==
734 PyTuple_GET_ITEM(view->defaults, NAMES_TYPE)) {
735 unique = get_unique(view, names.ptr());
736 if (!unique) {
! 737 iterator_clear(self);
! 738 return nullptr;
! 739 }
740 }
741 if (unique && unique.ptr() != Py_False) {
742 if (plain_row && prepare(view, row.ptr(), names.ptr(), unique.ptr()) < 0) {
! 743 iterator_clear(self);
! 744 return nullptr;
! 745 }
746 self->iterator = PyObject_GetIter(unique.ptr());
747 } else {
748 self->iterator = PyObject_CallOneArg(
749 PyTuple_GET_ITEM(view->defaults, FALLBACK_ITERATOR), names.ptr());Lines 748-763 748 self->iterator = PyObject_CallOneArg(
749 PyTuple_GET_ITEM(view->defaults, FALLBACK_ITERATOR), names.ptr());
750 }
751 if (!self->iterator) {
! 752 iterator_clear(self);
! 753 return nullptr;
! 754 }
755 if (!PyIter_Check(self->iterator)) {
! 756 PyErr_SetString(PyExc_TypeError, "Mapping key hook did not return an iterator");
! 757 iterator_clear(self);
! 758 return nullptr;
! 759 }
760 }
761 PyObject* value = PyIter_Next(self->iterator);
762 if (!value)
763 iterator_clear(self);Lines 765-779 765 }
766
767 inline PyObject* iterate(View* self) {
768 if (!self->defaults) {
! 769 PyErr_SetString(PyExc_AttributeError, "RowMapping has no row");
! 770 return nullptr;
! 771 }
772 auto* type = reinterpret_cast<PyTypeObject*>(PyTuple_GET_ITEM(self->defaults, ITERATOR_TYPE));
773 auto* iterator = reinterpret_cast<Iterator*>(type->tp_alloc(type, 0));
774 if (!iterator)
! 775 return nullptr;
776 Py_INCREF(self);
777 iterator->view = self;
778 return reinterpret_cast<PyObject*>(iterator);
779 }Lines 785-793 785
786 inline py::object make_base(const py::object& row_type, const py::object& names_type,
787 const py::object& fallback_iterator) {
788 if (!PyType_Check(row_type.ptr()) || !PyType_Check(names_type.ptr()))
! 789 throw py::type_error("Row and column names must be types");
790
791 static PyType_Slot iterator_slots[] = {
792 {Py_tp_dealloc, reinterpret_cast<void*>(iterator_dealloc)},
793 {Py_tp_traverse, reinterpret_cast<void*>(iterator_traverse)},Lines 801-813 801 Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, iterator_slots,
802 };
803 py::object iterator_type = steal(PyType_FromSpec(&iterator_spec));
804 if (!iterator_type)
! 805 throw py::error_already_set();
806 auto name = [](const char* text) {
807 py::object result = steal(PyUnicode_InternFromString(text));
808 if (!result)
! 809 throw py::error_already_set();
810 return result;
811 };
812 auto* row_class = reinterpret_cast<PyTypeObject*>(row_type.ptr());
813 py::object getattribute = name("__getattribute__");Lines 871-879 871 Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, slots,
872 };
873 py::object type = steal(PyType_FromSpec(&spec));
874 if (!type)
! 875 throw py::error_already_set();
876 type.attr("_row_mapping_defaults") = configuration;
877 type.attr("__slots__") = py::make_tuple("_row");
878 type.attr("_prepared_supported") = !owner.is_none();
879 return type;mssql_python/pybind/row_mapping_cache.hppLines 47-55 47 std::lock_guard<std::mutex> lock(entries.mutex);
48 for (Watcher* state = entries.head; state; state = state->next) {
49 if (state->interpreter == interpreter) {
50 if (state->epoch == std::numeric_limits<std::uint64_t>::max())
! 51 state->enabled = false;
52 else
53 ++state->epoch;
54 break;
55 }Lines 60-70 60
61 inline void destroy_watcher(PyObject* capsule) {
62 auto* state = static_cast<Watcher*>(PyCapsule_GetPointer(capsule, "mssql.RowMappingWatcher"));
63 if (!state) {
! 64 PyErr_WriteUnraisable(capsule);
! 65 return;
! 66 }
67 {
68 Registry& entries = registry();
69 std::lock_guard<std::mutex> lock(entries.mutex);
70 Watcher** link = &entries.head;Lines 68-86 68 Registry& entries = registry();
69 std::lock_guard<std::mutex> lock(entries.mutex);
70 Watcher** link = &entries.head;
71 while (*link && *link != state)
! 72 link = &(*link)->next;
73 if (*link)
74 *link = state->next;
75 }
76 if (Py_IsInitialized()) {
! 77 PyObject *error_type, *error_value, *error_traceback;
! 78 PyErr_Fetch(&error_type, &error_value, &error_traceback);
! 79 if (PyDict_ClearWatcher(state->id) < 0)
! 80 PyErr_WriteUnraisable(capsule);
! 81 PyErr_Restore(error_type, error_value, error_traceback);
! 82 }
83 delete state;
84 }
85
86 inline py::object watcher_owner() {Lines 85-127 85
86 inline py::object watcher_owner() {
87 PyObject* dictionary = PyInterpreterState_GetDict(PyInterpreterState_Get());
88 if (!dictionary) {
! 89 if (!PyErr_Occurred())
! 90 PyErr_SetString(PyExc_RuntimeError, "Cannot initialize RowMapping interpreter state");
! 91 throw py::error_already_set();
! 92 }
93 py::str key("_mssql_python_row_mapping_watcher");
94 PyObject* existing = PyDict_GetItemWithError(dictionary, key.ptr());
95 if (existing) {
96 if (existing != Py_None && !PyCapsule_IsValid(existing, "mssql.RowMappingWatcher"))
! 97 throw py::type_error("Invalid RowMapping watcher state");
98 return borrow(existing);
99 }
100 if (PyErr_Occurred())
! 101 throw py::error_already_set();
102 auto* state = new (std::nothrow) Watcher{PyInterpreterState_Get(), nullptr, 1, -1, true};
103 if (!state)
! 104 throw std::bad_alloc();
105 state->id = PyDict_AddWatcher(changed);
106 if (state->id < 0) {
107 delete state;
108 if (!PyErr_ExceptionMatches(PyExc_RuntimeError))
! 109 throw py::error_already_set();
110 PyErr_Clear();
111 if (PyErr_WarnEx(PyExc_RuntimeWarning,
112 "RowMapping preparation disabled: no CPython dict watcher available", 1) < 0)
! 113 throw py::error_already_set();
114 if (PyDict_SetItem(dictionary, key.ptr(), Py_None) < 0)
! 115 throw py::error_already_set();
116 return py::none();
117 }
118 py::object owner = steal(PyCapsule_New(state, "mssql.RowMappingWatcher", destroy_watcher));
119 if (!owner) {
! 120 PyDict_ClearWatcher(state->id);
! 121 delete state;
! 122 throw py::error_already_set();
! 123 }
124 {
125 Registry& entries = registry();
126 std::lock_guard<std::mutex> lock(entries.mutex);
127 state->next = entries.head;Lines 127-135 127 state->next = entries.head;
128 entries.head = state;
129 }
130 if (PyDict_SetItem(dictionary, key.ptr(), owner.ptr()) < 0)
! 131 throw py::error_already_set();
132 return owner;
133 }
134 #else
135 inline py::object watcher_owner() { return py::none(); }Lines 185-200 185 inline int allocate_entries(Schema* self, Py_ssize_t size) {
186 if (size == 0)
187 return 0;
188 if (size > PY_SSIZE_T_MAX / static_cast<Py_ssize_t>(sizeof(Entry))) {
! 189 PyErr_NoMemory();
! 190 return -1;
! 191 }
192 self->entries = static_cast<Entry*>(PyMem_Calloc(static_cast<size_t>(size), sizeof(Entry)));
193 if (!self->entries) {
! 194 PyErr_NoMemory();
! 195 return -1;
! 196 }
197 return 0;
198 }
199
200 inline int append_entry(Schema* self, PyObject* key, PyObject* index) {Lines 201-209 201 // Exact ints only: overflow is a fallback marker, not an early lookup error.
202 int overflow = 0;
203 long long offset = PyLong_AsLongLongAndOverflow(index, &overflow);
204 if (offset == -1 && PyErr_Occurred())
! 205 return -1;
206 Entry& entry = self->entries[self->entry_count];
207 entry.key = Py_NewRef(key);
208 entry.index = Py_NewRef(index);
209 entry.representable = overflow == 0 &&Lines 235-243 235 Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, slots,
236 };
237 py::object type = steal(PyType_FromSpec(&spec));
238 if (!type)
! 239 throw py::error_already_set();
240 return type;
241 }
242
243 inline bool builtin_keys(PyObject* dictionary, bool integer_values = false) {📋 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.connection.connection_pool.cpp: 81.3%
mssql_python.pybind.row_mapping_cache.hpp: 81.5%
mssql_python.pybind.connection.connection.cpp: 82.5%
mssql_python.logging.py: 86.2%
mssql_python.pybind.row_mapping.hpp: 86.8%
mssql_python.pooling.py: 90.1%🔗 Quick Links
|
Jahnvi Thakkar (jahnvi480)
changed the title
Jahnvi Thakkar (jahnvi480)
marked this pull request as ready for review
October 16, 2026 08:31
Sumit Sarabhai (sumitmsft)
approved these changes
Oct 19, 2026
Sumit Sarabhai (sumitmsft)
left a comment
Sumit Sarabhai (sumitmsft)
left a comment
Contributor
There was a problem hiding this comment.
No actionable findings.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The native heap types, GC/refcount ownership, and dictionary-watcher invalidation require rebuilt-extension lifetime and shutdown validation across supported Python versions that this review environment cannot provide.
No open issues 🎉
Review effort: Balanced
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.
Work Item / Issue Reference
Summary
This pull request introduces a new benchmark scenario for row-to-dictionary conversion and adds supporting infrastructure for efficient mapping in the codebase. The main changes include the addition of the
row_to_dictworkload, updates to validation logic to accommodate its unique characteristics, and the introduction of new C++ and Python classes to optimize and track row mapping schema. These changes enhance profiling coverage and prepare the system for more efficient dictionary mapping in result rows.Profiler Benchmark Enhancements:
Added a new benchmark scenario,
row_to_dict, which measures the warm-only conversion of 10,000 fetched rows with 32 integer columns to dictionaries usingdict(row._mapping). The benchmark is described in detail in theREADME.mdand implemented inworkloads.py. This scenario specifically times only the conversion step, not the fetch or metadata preparation, and allows for empty native diagnostics.Updated validation logic in both the controller and report modules to permit empty native (
cpp) diagnostics only for the newrow_to_dictscenario, and refactored phase validation into a shared functionvalidate_phases.Row Mapping Infrastructure:
Introduced a new
_ColumnNamesclass inrow.pyto encapsulate result column names, with validation and deduplication logic for mapping use. This class is now used in both sync and async cursor implementations to store column names, replacing plain tuples.Added a new C++ header,
row_mapping_cache.hpp, which implements a cache and watcher infrastructure for row mapping schema, supporting efficient dictionary mapping and change tracking in CPython 3.12+.Exposed a new C++ binding
_make_row_mapping_baseand included the newrow_mapping.hppheader in the bindings module to support future extensions for fast row mapping.