perf: add comprehensive benchmark suite with CI regression detection#376
Open
dlevy-msft-sql wants to merge 2 commits into
Open
perf: add comprehensive benchmark suite with CI regression detection#376dlevy-msft-sql wants to merge 2 commits into
dlevy-msft-sql wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #376 +/- ##
===========================================
+ Coverage 80.70% 96.60% +15.89%
===========================================
Files 35 92 +57
Lines 6910 74351 +67441
===========================================
+ Hits 5577 71826 +66249
- Misses 1064 2190 +1126
- Partials 269 335 +66
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
de4feca to
34fa0f6
Compare
Add 96 benchmarks covering core driver hot paths: - Token parsing (10): ParseInfo, ParseDone, ParseReturnStatus, etc. - TDS buffer I/O (10): Read/Write in small/medium/large sizes - String encoding (10): Str2ucs2, Ucs22str, ManglePassword - Type conversion (13): ConvertAssign for all common type pairs - Wire protocol types (17): ReadFixedType, ReadByteLenType, etc. - RPC encoding (7): SendRpc with various parameter types - Bulk copy params (9): BulkMakeParam for common column types - Integration round-trips (14): Full E2E through SQL Server - Connection string parsing (5): URL and ADO format variants CI integration (.github/workflows/pr-validation.yml): - Runs benchmarks on every PR with SQL Server in Docker - Compares PR branch against main using benchstat - Uses -count=10 and -alpha=0.01 for statistical rigor - Fails CI on statistically significant regressions (p<0.01) - Reports improvements via GitHub Actions notices - Explicit benchmark pattern avoids slow pre-existing benchmarks - Copies benchmark files to main worktree for fair comparison Also fixes a pre-existing deadlock in BenchmarkSelectWithTypeMismatch where defer rows.Close() inside a loop with MaxOpenConns=1 caused connection starvation.
34fa0f6 to
a216232
Compare
- Extract common Parse benchmark loop into benchmarkParse helper - Add BenchmarkRoundTrip_MessageQuery covering the sqlexp message-based query loop
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.
Problem
The driver has no systematic benchmark coverage and no CI guardrails against performance regressions. Changes that degrade hot-path performance can merge undetected.
Solution
Add 96 benchmarks covering all core driver hot paths, with automated CI regression detection using benchstat.
Benchmarks Added
token_benchmark_test.gobuf_benchmark_test.gotds_benchmark_test.goconvert_benchmark_test.gotypes_benchmark_test.gorpc_benchmark_test.gobulkcopy_benchmark_test.gointegration_benchmark_test.gomsdsn/conn_str_benchmark_test.goCI Integration
benchmarksjob in GitHub Actions with SQL Server 2025 in Docker-benchtime=100ms -count=1) stabilizes CPU caches and turbo boost before measurement; results are discardedmainvia git worktree, then PR benchmarks, both with-count=10 -benchtime=1sbenchstat -alpha=0.01(p < 0.01 significance threshold)::noticeannotationsTdsBuffer_Write_Largefrom regression gate (cache-sensitive ~120ns operation with 30-46% natural variance)Bug Fix
Fixes a pre-existing deadlock in
BenchmarkSelectWithTypeMismatchwheredefer rows.Close()inside a loop withMaxOpenConns=1caused connection starvation. Movedrows.Close()to end of loop body.Testing
go test -run='^$' -bench=. -benchtime=1s . ./msdsn)go test ./msdsn ./internal/... ./integratedauth ./azuread)