PERF: Add pooled lifecycle scenarios to PR reports - #821
Gaurav Sharma (bewithgaurav) wants to merge 1 commit into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Performance Report✅ No regression detectedNo consistent slowdowns detected across all 2 environments. 0 IMPROVEMENTS 0 SLOWDOWNS 2/2 ENVIRONMENTS Coverage: 2 of 2 environments completed. Advisory result; does not block merging. Performance diagnosticsPhase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed. No affected phases or call-count changes were recorded. 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 |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Three moderate review findings remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds sequential and concurrent prewarmed pooled connection lifecycle benchmarks to PR performance reports.
Changes:
- Adds pooled checkout/check-in workloads.
- Registers and reports both scenarios.
- Adds orchestration tests and documentation.
| File | Reviewed changes |
|---|---|
tests/test_036_profiler_ci.py |
Tests workload registration and execution contracts; per-worker call distribution needs stronger validation. |
profiler/core.py |
Routes pooled scenarios through connection-string execution. |
eng/profiler_benchmarks/workloads.py |
Implements sequential and concurrent lifecycle benchmarks; timing instrumentation and repeated imports require correction. |
eng/profiler_benchmarks/report.py |
Adds scenario names and report labels. |
eng/profiler_benchmarks/README.md |
Documents the expanded benchmark suite. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _connect_close(connection_string): | ||
| from mssql_python import connect | ||
|
|
||
| connect(connection_string).close() |
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 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
|
Work Item / Issue Reference
Summary
Adds two prewarmed pooled connection lifecycle scenarios to the PR Performance Report:
Both measure
Connection.close()and exclude physical-connection and thread startup from the timed window. This catches fixed lifecycle costs that the existing single-connect scenario misses and aggregates sub-millisecond per-connection changes above the report's 1 ms absolute floor.The scenarios add approximately 16 seconds per SQL leg across the report's warmup and five measured base/candidate pairs. SQL Server 2022 and 2025 legs run in parallel.
Validation on a local macOS profiling build against SQL Server 2022: