CHORE: Remove macOS from routine PR Performance Reports - #800
Merged
Gaurav Sharma (bewithgaurav) merged 3 commits intoSep 18, 2026
Merged
Gaurav Sharma (bewithgaurav) merged 3 commits into
Gaurav Sharma (bewithgaurav) merged 3 commits into
Conversation
Keep functional macOS validation while using stable Ubuntu measurements as the Unix performance signal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Performance ReportPerformance assessment pending. Waiting for the matching performance run for head |
Copilot started reviewing on behalf of
Gaurav Sharma (bewithgaurav)
September 18, 2026 08:08
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review comments were identified, and the supplied assessment finds it ready for approval.
Pull request overview
Removes hosted macOS from routine performance reporting while retaining macOS functional validation and using Ubuntu/Linux as the Unix signal.
Changes:
- Reduces performance reporting from five environments to three.
- Removes macOS profiling setup, benchmarks, and artifacts.
- Updates reports, documentation, CI configuration, and contract tests.
File summaries
| File | Summary |
|---|---|
tests/test_036_profiler_ci.py |
Updates profiler contract tests for three environments. |
eng/profiler_benchmarks/report.py |
Defines the new legs and Unix display label. |
eng/profiler_benchmarks/README.md |
Documents the revised publication contract. |
eng/pipelines/pr-validation-pipeline.yml |
Removes macOS profiling while retaining macOS validation and Ubuntu profiling. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Use the existing Ubuntu SQL Server 2025 matrix leg to complete routine Windows and Unix version coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Gaurav Sharma (bewithgaurav)
September 18, 2026 08:13
View session
Gaurav Sharma (bewithgaurav)
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Remove the unintended fourth reporting environment and clarify the macOS coverage comment.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
eng/pipelines/pr-validation-pipeline.yml:662
- This newly added matrix leg creates
profiler-Linux-SQL2025, which is the extra fourth environment contrary to the PR's stated three-environment contract. It also adds a second 100-minute Unix profiler run; remove the profiling leg/publication for SQL Server 2025 if the three-environment requirement is authoritative.
Ubuntu_SQL2025:
dockerImage: 'ubuntu:24.04'
distroName: 'Ubuntu-SQL2025'
sqlServerImage: 'mcr.microsoft.com/mssql/server:2025-latest'
useAzureSQL: 'false'
profilerLeg: 'Linux-SQL2025'
eng/pipelines/pr-validation-pipeline.yml:632
- The new comment says functional macOS coverage “remains above,” which is grammatically incomplete and unclear. Use “remains in place” (or another phrase that states the intended meaning).
# Routine PR profiling excludes hosted macOS because Colima-backed control
# runs produced regressions with no product-code change. Functional macOS
# coverage remains above; Unix performance signal comes from Ubuntu.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
Jahnvi Thakkar (jahnvi480)
approved these changes
Sep 18, 2026
Jahnvi Thakkar (jahnvi480)
left a comment
Jahnvi Thakkar (jahnvi480)
left a comment
Contributor
There was a problem hiding this comment.
LGTM! Approving
Gaurav Sharma (bewithgaurav)
enabled auto-merge (squash)
September 18, 2026 08:45
📊 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: 64.1%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.row.py: 77.6%
mssql_python.pybind.ddbc_bindings.cpp: 77.7%
mssql_python.pybind.connection.connection_pool.cpp: 81.8%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.pybind.py_type_cache.hpp: 91.6%🔗 Quick Links
|
gargsaumya
approved these changes
Sep 18, 2026
Copilot started reviewing on behalf of
Gaurav Sharma (bewithgaurav)
September 18, 2026 09:05
View session
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
Remove hosted macOS from routine PR Performance Reports after the documentation-only control PR reported a false 32.9% macOS slowdown.
The trusted base for this PR still expects the previous five-environment contract, so this PR's own performance report may remain unavailable. Subsequent PRs will use the new four-environment contract.
Validation
106 profiler CI contract tests passed. YAML parsing, Black, Flake8, and diff checks passed.