Skip to content

AI: Strengthen Perf Police investigation and evidence guidance - #835

Open
Jahnvi Thakkar (jahnvi480) wants to merge 3 commits into
mainfrom
jahnvi/perf-police-investigation-guidance
Open

Jahnvi Thakkar (jahnvi480) wants to merge 3 commits into
mainfrom
jahnvi/perf-police-investigation-guidance

Conversation

@jahnvi480

@jahnvi480 Jahnvi Thakkar (jahnvi480) commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#48077

GitHub Issue: N/A


Summary

Strengthen the reusable Perf Police agent and performance-review/profiling skills with a generic, evidence-led investigation method rather than PR-specific case studies.

  • Define the problem, trace actual dispatch, test falsifiable explanations, and assess the smallest fix that preserves surrounding behavior.
  • Preserve ownership, diagnostics, Python customization, converter behavior, invalidation, and cleanup when simplifying hot paths.
  • Distinguish fixture and harness failures from driver defects, and preserve meaningful coverage when consolidating tests.
  • Verify effective Release builds and native imports; separate source checks, native correctness, profiling attribution, and uninstrumented latency. Keep paired statistics, pyodbc comparisons, and CI provenance explicit.
  • Keep execution proportionate: clear review-to-coding handoffs, one shared-runtime owner, bounded resources, verified cleanup, and no redundant approval rounds within an authorized scope.

Changes are limited to three agent/skill Markdown files. No driver, benchmark, pipeline, workload, or threshold implementation changes are included.

Validation: git diff --check passed. Frontmatter, relative links/anchors, ASCII text, and code fences were checked. Native builds and SQL tests were not run because this is a documentation-only change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 14:24
@github-actions github-actions Bot added the pr-size: medium Moderate update size label Oct 8, 2026
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

PR Performance Report

✅ No regression detected

No 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 diagnostics

Phase 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 timings

Unix / SQL Server 2022

Database task Before After Paired change Result
Connection opening 10.507 ms 10.793 ms +1.5% no signal
SELECT queries 1.080 ms 1.073 ms -0.4% no signal
Row insertion 35.064 ms 34.757 ms -0.3% no signal
Executemany inserts 157.926 ms 159.911 ms +1.3% no signal
Fetch-all queries 121.455 ms 122.016 ms +0.3% no signal
Row-by-row fetching 14.547 ms 14.340 ms -2.1% no signal
Batched row fetching 118.801 ms 117.970 ms -0.8% no signal
Transaction commit and rollback 118.116 ms 116.775 ms -1.9% no signal
Arrow row fetching 94.096 ms 95.363 ms +2.4% no signal
100,000-row insertion 445.588 ms 452.007 ms +0.9% no signal
Row fetching in batches of 100 121.157 ms 121.313 ms +0.6% no signal
Row fetching in batches of 10,000 139.375 ms 139.085 ms -1.1% no signal
Repeated positional queries 34.800 ms 34.244 ms -0.8% no signal
Repeated named-parameter queries 44.641 ms 36.442 ms -20.3% no signal
Legacy 100,000-row insertion 356.455 ms 357.069 ms -0.2% no signal
Insertion with explicit input sizes 489.118 ms 500.708 ms +2.2% no signal
Joined aggregation queries 177.374 ms 181.819 ms +1.9% no signal
Large joined-result fetching 185.880 ms 187.600 ms +1.3% no signal
1.2-million-row fetching 3474.595 ms 3474.914 ms -0.7% no signal
Common table expression queries 5.303 ms 5.373 ms +0.6% no signal
256 KiB VARCHAR(MAX) / fetchall() 1.307 ms 1.297 ms -4.8% no signal
10,000 scalar values / fetchval() (debug disabled) 106.865 ms 106.357 ms +0.2% no signal

Unix / SQL Server 2025

Database task Before After Paired change Result
Connection opening 97.183 ms 97.978 ms +0.9% no signal
SELECT queries 1.097 ms 1.069 ms -3.0% no signal
Row insertion 36.108 ms 34.718 ms -3.6% no signal
Executemany inserts 155.467 ms 155.594 ms +0.1% no signal
Fetch-all queries 123.470 ms 123.709 ms -0.3% no signal
Row-by-row fetching 14.430 ms 14.621 ms +1.0% no signal
Batched row fetching 118.201 ms 118.383 ms -0.7% no signal
Transaction commit and rollback 115.901 ms 115.784 ms +0.8% no signal
Arrow row fetching 94.186 ms 93.681 ms -0.5% no signal
100,000-row insertion 448.993 ms 439.024 ms -2.3% no signal
Row fetching in batches of 100 122.957 ms 122.278 ms -1.4% no signal
Row fetching in batches of 10,000 150.733 ms 142.620 ms -6.1% no signal
Repeated positional queries 35.622 ms 33.691 ms -5.1% no signal
Repeated named-parameter queries 37.537 ms 36.287 ms -4.9% no signal
Legacy 100,000-row insertion 373.711 ms 352.392 ms +0.1% no signal
Insertion with explicit input sizes 511.162 ms 499.259 ms +0.2% no signal
Joined aggregation queries 160.688 ms 162.135 ms +0.6% no signal
Large joined-result fetching 250.797 ms 193.742 ms -7.2% no signal
1.2-million-row fetching 3605.495 ms 3586.879 ms +0.1% no signal
Common table expression queries 5.339 ms 5.430 ms +1.3% no signal
256 KiB VARCHAR(MAX) / fetchall() 1.477 ms 1.483 ms -1.9% no signal
10,000 scalar values / fetchval() (debug disabled) 107.380 ms 107.616 ms -0.0% no signal
Build and measurement details

ADO build 182109

PR head: 747af0d2d45419deffbb9308c0bdbb96775c7fa1
Base: 0facea305789bdec35d2be8e29346d20bcdb6711
Measured merge: 13c45c9e73cfddb98acb79dc65bff7613ff24082

  • 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The documentation-only changes are consistent, scoped, and contain no identified blocking issues.

0 open findings

What changed in this PR

Strengthens Perf Police guidance with evidence-led review, profiling, and handoff practices.

Changes:

  • Adds a falsifiable, baseline-driven performance review workflow.
  • Expands profiling provenance, workload, Release-build, and statistical guidance.
  • Clarifies the Perf Police agent’s review-only role and implementation handoff.
File Description
.github/​skills/​performance-code-review/​SKILL.md Expands performance review and evidence requirements.
.github/​skills/​mssql-profiler/​SKILL.md Strengthens profiling execution and reporting guidance.
.github/​agents/​perf-police.agent.md Clarifies workflow use and coding-session handoff.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

85%


📈 Total Lines Covered: 9445 out of 11094
📁 Project: mssql-python


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

No 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.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%
mssql_python.cursor.py: 92.5%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 14:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The documentation-only changes are consistent, actionable, and have valid local links and anchors.

0 open findings

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@jahnvi480
Jahnvi Thakkar (jahnvi480) marked this pull request as ready for review October 12, 2026 04:35
Copilot AI balanced review requested due to automatic review settings October 12, 2026 06:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The documented Unix build commands still do not reliably produce the required Release binaries.

1 open finding

🧠 Review effort: Balanced

Comment on lines +98 to +102
For single-configuration CMake generators, `--config Release` alone does not
select an optimized build; inspect `CMAKE_BUILD_TYPE` and the effective compile
commands. Verify optimization, assertion, and profiling flags on every measured
arm, including archived baselines. Retain that evidence rather than accepting
or rejecting a build solely from its command label.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants