From aeef2aaba060d0688a22805063fadf70c441a16d Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 24 Sep 2026 12:41:11 +0530 Subject: [PATCH 1/5] CHORE: Add multi-chunk LOB fetch coverage to PR performance reports Exercise VARCHAR(MAX), NVARCHAR(MAX), and VARBINARY(MAX) at 64 and 256 KiB through fetchone, fetchmany(1), and fetchall. Validate exact payloads outside the timed fetch window and expose available diagnostic-call counts without treating missing instrumentation as zero. Preserve existing thresholds, advisory behavior, and platform selection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eng/profiler_benchmarks/README.md | 31 +++++++- eng/profiler_benchmarks/report.py | 25 +++++++ eng/profiler_benchmarks/workloads.py | 52 +++++++++++++ tests/test_036_profiler_ci.py | 105 ++++++++++++++++++++++++++- 4 files changed, 208 insertions(+), 5 deletions(-) diff --git a/eng/profiler_benchmarks/README.md b/eng/profiler_benchmarks/README.md index 689fc5b29..fe4c02c9a 100644 --- a/eng/profiler_benchmarks/README.md +++ b/eng/profiler_benchmarks/README.md @@ -12,9 +12,33 @@ python -m eng.profiler_benchmarks.controller --base main --candidate HEAD \ python -m eng.profiler_benchmarks.report profiler-results/report.json ``` -The fixed registry has 20 tasks. `--scenarios` runs a local subset, but subset +The fixed registry has 38 tasks. `--scenarios` runs a local subset, but subset reports remain incomplete and cannot produce a verdict. +## Large-value fetch coverage + +Eighteen tasks fetch one 64 KiB or 256 KiB value from `VARCHAR(MAX)`, +`NVARCHAR(MAX)`, or `VARBINARY(MAX)` through `fetchone()`, `fetchmany(1)`, and +`fetchall()`. Unlike a million short rows, these values require LOB continuation +calls. Task names have the form `lob_varchar_256k_fetchone`. + +Payload sizes describe SQL data bytes (NVARCHAR uses two bytes per BMP character), +not row counts or a promise about the driver's internal chunk count. Unicode text +and binary embedded NULs are checked. Query execution/setup and exact payload/type +validation are outside the timed fetch window. An unexpected warning or truncated +value fails the workload rather than producing a successful performance verdict. + +The existing elapsed-time thresholds remain unchanged. Available +`SQLGetDiagRec` profiler counters are supporting diagnostics, not a separate gate; +an absent counter is reported as unavailable, not zero. The old +`SQLGetAllDiagRecords` helper count is not equivalent to the number of underlying +ODBC calls. Mixed-warning preservation belongs in functional driver regressions, +not these clean-payload timings. + +Both revisions run the same new workloads, so the first comparison can include a +base that predates them. Older artifacts missing these tasks remain incomplete or +invalid; they must not produce a full-coverage verdict. + ## Measurement contract CI uses the PR merge's first parent as the exact base. It reuses the @@ -39,6 +63,11 @@ is published. A failed aggregate build can still publish usable profiler artifac Exact-head reports may finalize after merge; stale heads are ignored. Missing, malformed, canceled, incomplete, or invalid data remains unavailable. +The two reported environments are Linux/SQL Server combinations, not all supported +operating systems. A clean report cannot rule out a macOS- or Windows-specific +LOB slowdown. Changes to streaming/diagnostic code still need targeted release-build +measurements and warning-preservation checks on those platforms. + The report highlights consistent slowdowns and improvements using the same 20% median change, 1 ms absolute change, and 80% pair-agreement requirements. diff --git a/eng/profiler_benchmarks/report.py b/eng/profiler_benchmarks/report.py index 6188894cc..f9c6ac531 100644 --- a/eng/profiler_benchmarks/report.py +++ b/eng/profiler_benchmarks/report.py @@ -38,6 +38,17 @@ "fetch_1_2m": "1.2-million-row fetching", "cte": "Common table expression queries", } +TASK_NAMES.update( + { + f"lob_{sql_type}_{size_kib}k_{api}": ( + f"{size_kib} KiB {sql_type.upper()}(MAX) / " + f"{'fetchmany(1)' if api == 'fetchmany' else api + '()'}" + ) + for sql_type in ("varchar", "nvarchar", "varbinary") + for size_kib in (64, 256) + for api in ("fetchone", "fetchmany", "fetchall") + } +) CASES = tuple(TASK_NAMES) MAX_BYTES = 8 * 1024 * 1024 MAX_COMMENT_CHARS = 60000 @@ -294,6 +305,20 @@ def comparisons(report): before = [s[layer].get(label) for s in base] after = [s[layer].get(label) for s in candidate] if not all(before) or not all(after): + if name.startswith("lob_") and "SQLGetDiagRec" in label: + # Missing instrumentation is not evidence of zero driver calls. + old_calls = ( + f"{statistics.median(s['calls'] for s in before):g}" + if all(before) + else "unavailable" + ) + new_calls = ( + f"{statistics.median(s['calls'] for s in after):g}" + if all(after) + else "unavailable" + ) + changed_counts.append(f"{label} ({old_calls} -> {new_calls} calls)") + continue changed_counts.append(f"{label} (added, removed, or intermittent)") continue before_calls = statistics.median(s["calls"] for s in before) diff --git a/eng/profiler_benchmarks/workloads.py b/eng/profiler_benchmarks/workloads.py index 324600860..0045be8d2 100644 --- a/eng/profiler_benchmarks/workloads.py +++ b/eng/profiler_benchmarks/workloads.py @@ -133,6 +133,51 @@ def legacy_insertmany(conn, ctx, input_sizes=False): conn.rollback() +def lob_fetch(conn, ctx, sql_type, payload_bytes, api): + """Fetch one multi-chunk value; setup and exact-value validation are not timed.""" + if sql_type == "nvarchar": + expression = f"REPLICATE(CAST(NCHAR(233) AS NVARCHAR(MAX)), {payload_bytes // 2})" + expected = "\u00e9" * (payload_bytes // 2) + elif sql_type == "varbinary": + expression = ( + "CONVERT(VARBINARY(MAX), " + f"REPLICATE(CAST(CHAR(0) + 'x' AS VARCHAR(MAX)), {payload_bytes // 2}))" + ) + expected = b"\x00x" * (payload_bytes // 2) + else: + expression = f"REPLICATE(CAST('x' AS VARCHAR(MAX)), {payload_bytes})" + expected = "x" * payload_bytes + + with conn.cursor() as cursor: + cursor.execute(f"SELECT {expression} AS payload") + ctx.enable() + try: + start = time.perf_counter() + if api == "fetchone": + row = cursor.fetchone() + elif api == "fetchmany": + rows = cursor.fetchmany(1) + else: + rows = cursor.fetchall() + wall_ms = (time.perf_counter() - start) * 1000 + cpp, py = ctx.collect() + if api != "fetchone": + assert len(rows) == 1 + row = rows[0] + assert row is not None and len(row) == 1 + assert type(row[0]) is type(expected) and row[0] == expected + assert not cursor.messages, "Clean LOB fetch unexpectedly produced diagnostics" + return dict( + title="Multi-chunk LOB fetch", + wall_ms=wall_ms, + cpp=cpp, + py=py, + detail=f"Rows: 1; type: {sql_type}; payload bytes: {payload_bytes}; API: {api}", + ) + finally: + ctx.disable() + + def registry(): """Keep every PR #552 scenario, including its existing timing boundaries.""" result = dict(scenarios.SCENARIOS) @@ -145,4 +190,11 @@ def registry(): setinputsizes=(partial(legacy_insertmany, input_sizes=True), False), ) result.update((name, (partial(query, sql=sql), False)) for name, sql in QUERIES.items()) + for sql_type in ("varchar", "nvarchar", "varbinary"): + for size_kib in (64, 256): + for api in ("fetchone", "fetchmany", "fetchall"): + result[f"lob_{sql_type}_{size_kib}k_{api}"] = ( + partial(lob_fetch, sql_type=sql_type, payload_bytes=size_kib * 1024, api=api), + False, + ) return result diff --git a/tests/test_036_profiler_ci.py b/tests/test_036_profiler_ci.py index 5bbf99f86..24e93d440 100644 --- a/tests/test_036_profiler_ci.py +++ b/tests/test_036_profiler_ci.py @@ -116,7 +116,7 @@ def test_consistent_slowdown_is_advisory_regression(report): ) body = reporting.render([report], "c" * 40, 42) assert "### ⚠️ Performance regression detected" in body - assert "20 database tasks consistently slowed down" in body + assert f"{len(reporting.CASES)} database tasks consistently slowed down" in body assert "| Unix / SQL Server 2022 | Connection opening |" in body assert "Unavailable: Unix / SQL Server 2025 (incomplete benchmark)." in body assert body.index("consistently slowed down") < body.index( @@ -248,7 +248,12 @@ def test_render_bounds_schema_valid_diagnostics(report): reporting.validate(item) body = reporting.render(reports, "c" * 40, 42) assert len(body) <= 60000 - assert "20 additional diagnostic rows are available in the raw ADO artifacts" in body + extra = len(reporting.CASES) * len(reporting.LEGS) - reporting.MAX_DIAGNOSTIC_ROWS + total = len(reporting.CASES) * len(reporting.LEGS) + assert ( + f"{extra} additional diagnostic rows are available in the raw ADO artifacts" in body + or f"{total} diagnostic rows are available in the raw ADO artifacts" in body + ) assert "All database tasks and timings" in body assert "Build and measurement details" in body @@ -684,6 +689,94 @@ def test_report_cases_match_the_executed_workload_registry(): assert tuple(workloads.registry()) == reporting.CASES +@pytest.mark.parametrize("sql_type", ("varchar", "nvarchar", "varbinary")) +@pytest.mark.parametrize("size", (8190, 8192, 8194, 65536, 262144)) +@pytest.mark.parametrize("api", ("fetchone", "fetchmany", "fetchall")) +def test_lob_workload_validates_payload_and_times_only_fetch(sql_type, size, api, monkeypatch): + expected = ( + b"\x00x" * (size // 2) + if sql_type == "varbinary" + else "\u00e9" * (size // 2) if sql_type == "nvarchar" else "x" * size + ) + cursor = MagicMock() + cursor.fetchone.return_value = (expected,) + cursor.fetchmany.return_value = [(expected,)] + cursor.fetchall.return_value = [(expected,)] + cursor.messages = [] + connection = MagicMock() + connection.cursor.return_value.__enter__.return_value = cursor + context = MagicMock() + context.collect.return_value = ({}, {}) + + def enable(): + cursor.execute.assert_called_once() + for method in ("fetchone", "fetchmany", "fetchall"): + getattr(cursor, method).assert_not_called() + + context.enable.side_effect = enable + monkeypatch.setattr(benchmark_workloads.time, "perf_counter", MagicMock(side_effect=[1, 1.1])) + result = benchmark_workloads.lob_fetch(connection, context, sql_type, size, api) + assert "(MAX)" in cursor.execute.call_args.args[0] + assert result["wall_ms"] == pytest.approx(100) + assert result["detail"] == f"Rows: 1; type: {sql_type}; payload bytes: {size}; API: {api}" + getattr(cursor, api).assert_called_once_with(*((1,) if api == "fetchmany" else ())) + for other in {"fetchone", "fetchmany", "fetchall"} - {api}: + getattr(cursor, other).assert_not_called() + context.collect.assert_called_once() + context.disable.assert_called_once() + + +@pytest.mark.parametrize( + "problem", ("truncated", "wrong-type", "missing", "extra", "warning", "error") +) +def test_lob_workload_rejects_invalid_results_and_always_disables(problem): + cursor = MagicMock() + cursor.fetchall.return_value = [("x" * 65536,)] + cursor.messages = [] + if problem == "truncated": + cursor.fetchall.return_value = [("x" * 65535,)] + elif problem == "wrong-type": + cursor.fetchall.return_value = [(b"x" * 65536,)] + elif problem == "missing": + cursor.fetchall.return_value = [] + elif problem == "extra": + cursor.fetchall.return_value *= 2 + elif problem == "warning": + cursor.messages = [("01000", "unexpected")] + else: + cursor.fetchall.side_effect = RuntimeError("fetch failed") + connection = MagicMock() + connection.cursor.return_value.__enter__.return_value = cursor + context = MagicMock() + context.collect.return_value = ({}, {}) + with pytest.raises(RuntimeError if problem == "error" else AssertionError): + benchmark_workloads.lob_fetch(connection, context, "varchar", 65536, "fetchall") + context.disable.assert_called_once() + + +def test_lob_regression_reports_native_diagnostic_counts_without_assuming_missing_is_zero(report): + name = "lob_varchar_256k_fetchone" + label = "ddbc::AppendDiagRecords::SQLGetDiagRec_call" + for pair in report["pairs"]: + for scenario in pair["candidate"]["scenarios"].values(): + scenario["wall_ms"] = 10 + scenario = pair["candidate"]["scenarios"][name] + scenario["wall_ms"] = 100 + scenario["cpp"][label] = dict(calls=128, total_us=1280, min_us=10, max_us=10) + reporting.validate(report) + result = next(row for row in reporting.comparisons(report) if row["name"] == name) + assert result["status"] == "regression" + assert f"{label} (unavailable -> 128 calls)" in result["counts"] + body = reporting.render([report], "c" * 40, 42) + assert "Performance regression detected" in body + assert "256 KiB VARCHAR(MAX)" in body + for pair in report["pairs"]: + pair["candidate"]["scenarios"][name]["wall_ms"] = 10 + assert ( + next(row for row in reporting.comparisons(report) if row["name"] == name)["status"] == "ok" + ) + + def test_query_workload_executes_and_collects(monkeypatch): cursor = MagicMock() cursor.fetchall.return_value = [(1,), (2,)] @@ -1243,12 +1336,16 @@ def corrupt_deflate(raw): assert "### Unix / SQL Server 2025" in posted[1] assert reporting.escape("Linux-SQL2022 (invalid artifact)") in posted[1] assert "Unavailable: Unix / SQL Server 2022 (invalid artifact)." in posted[1] - assert posted[1].count("20 database tasks consistently slowed down") == 1 + assert ( + posted[1].count(f"{len(reporting.CASES)} database tasks consistently slowed down") == 1 + ) else: assert "**Coverage:** 2 of 2 environments completed." in posted[1] assert "### Unix / SQL Server 2022" in posted[1] assert "### Unix / SQL Server 2025" in posted[1] - assert posted[1].count("20 database tasks consistently slowed down") == 1 + assert ( + posted[1].count(f"{len(reporting.CASES)} database tasks consistently slowed down") == 1 + ) def test_publisher_waits_for_newer_run_after_exact_head_build_is_canceled(report, monkeypatch): From db87e19235aeaccc949dd13070e265d83a8f7436 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 24 Sep 2026 12:49:20 +0530 Subject: [PATCH 2/5] CHORE: Limit routine LOB profiling to one representative fetch Keep one 256 KiB VARCHAR(MAX) fetchall workload, bringing the fixed registry to 21 tasks. Remove the unused type, size and API matrix while retaining exact payload checks and diagnostic-call visibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eng/profiler_benchmarks/README.md | 21 ++++++++------ eng/profiler_benchmarks/report.py | 12 +------- eng/profiler_benchmarks/workloads.py | 39 ++++++-------------------- tests/test_036_profiler_ci.py | 41 ++++++++++++---------------- 4 files changed, 41 insertions(+), 72 deletions(-) diff --git a/eng/profiler_benchmarks/README.md b/eng/profiler_benchmarks/README.md index fe4c02c9a..b63ec107c 100644 --- a/eng/profiler_benchmarks/README.md +++ b/eng/profiler_benchmarks/README.md @@ -12,19 +12,19 @@ python -m eng.profiler_benchmarks.controller --base main --candidate HEAD \ python -m eng.profiler_benchmarks.report profiler-results/report.json ``` -The fixed registry has 38 tasks. `--scenarios` runs a local subset, but subset +The fixed registry has 21 tasks. `--scenarios` runs a local subset, but subset reports remain incomplete and cannot produce a verdict. ## Large-value fetch coverage -Eighteen tasks fetch one 64 KiB or 256 KiB value from `VARCHAR(MAX)`, -`NVARCHAR(MAX)`, or `VARBINARY(MAX)` through `fetchone()`, `fetchmany(1)`, and -`fetchall()`. Unlike a million short rows, these values require LOB continuation -calls. Task names have the form `lob_varchar_256k_fetchone`. +One task, `lob_varchar_256k_fetchall`, fetches one 256 KiB `VARCHAR(MAX)` value +through `fetchall()`. Unlike a million short rows, this value requires LOB +continuation calls. Unicode/binary variants, other fetch APIs, and chunk-boundary +combinations belong in functional regression coverage or targeted performance +investigations rather than multiplying routine CI tasks. -Payload sizes describe SQL data bytes (NVARCHAR uses two bytes per BMP character), -not row counts or a promise about the driver's internal chunk count. Unicode text -and binary embedded NULs are checked. Query execution/setup and exact payload/type +The payload size describes SQL data bytes, not row counts or a promise about the +driver's internal chunk count. Query execution/setup and exact payload/type validation are outside the timed fetch window. An unexpected warning or truncated value fails the workload rather than producing a successful performance verdict. @@ -39,6 +39,11 @@ Both revisions run the same new workloads, so the first comparison can include a base that predates them. Older artifacts missing these tasks remain incomplete or invalid; they must not produce a full-coverage verdict. +The added task runs 24 times across routine CI: two revisions, six pairs including +warmup, and two environments. It needs no benchmark table or additional build. +Measure its incremental duration on each runner; do not infer the cost from task +count alone. + ## Measurement contract CI uses the PR merge's first parent as the exact base. It reuses the diff --git a/eng/profiler_benchmarks/report.py b/eng/profiler_benchmarks/report.py index f9c6ac531..49888aa4f 100644 --- a/eng/profiler_benchmarks/report.py +++ b/eng/profiler_benchmarks/report.py @@ -37,18 +37,8 @@ "large_fetch": "Large joined-result fetching", "fetch_1_2m": "1.2-million-row fetching", "cte": "Common table expression queries", + "lob_varchar_256k_fetchall": "256 KiB VARCHAR(MAX) / fetchall()", } -TASK_NAMES.update( - { - f"lob_{sql_type}_{size_kib}k_{api}": ( - f"{size_kib} KiB {sql_type.upper()}(MAX) / " - f"{'fetchmany(1)' if api == 'fetchmany' else api + '()'}" - ) - for sql_type in ("varchar", "nvarchar", "varbinary") - for size_kib in (64, 256) - for api in ("fetchone", "fetchmany", "fetchall") - } -) CASES = tuple(TASK_NAMES) MAX_BYTES = 8 * 1024 * 1024 MAX_COMMENT_CHARS = 60000 diff --git a/eng/profiler_benchmarks/workloads.py b/eng/profiler_benchmarks/workloads.py index 0045be8d2..c183f5d89 100644 --- a/eng/profiler_benchmarks/workloads.py +++ b/eng/profiler_benchmarks/workloads.py @@ -133,37 +133,22 @@ def legacy_insertmany(conn, ctx, input_sizes=False): conn.rollback() -def lob_fetch(conn, ctx, sql_type, payload_bytes, api): +def lob_fetch(conn, ctx): """Fetch one multi-chunk value; setup and exact-value validation are not timed.""" - if sql_type == "nvarchar": - expression = f"REPLICATE(CAST(NCHAR(233) AS NVARCHAR(MAX)), {payload_bytes // 2})" - expected = "\u00e9" * (payload_bytes // 2) - elif sql_type == "varbinary": - expression = ( - "CONVERT(VARBINARY(MAX), " - f"REPLICATE(CAST(CHAR(0) + 'x' AS VARCHAR(MAX)), {payload_bytes // 2}))" - ) - expected = b"\x00x" * (payload_bytes // 2) - else: - expression = f"REPLICATE(CAST('x' AS VARCHAR(MAX)), {payload_bytes})" - expected = "x" * payload_bytes + payload_bytes = 256 * 1024 + expression = f"REPLICATE(CAST('x' AS VARCHAR(MAX)), {payload_bytes})" + expected = "x" * payload_bytes with conn.cursor() as cursor: cursor.execute(f"SELECT {expression} AS payload") ctx.enable() try: start = time.perf_counter() - if api == "fetchone": - row = cursor.fetchone() - elif api == "fetchmany": - rows = cursor.fetchmany(1) - else: - rows = cursor.fetchall() + rows = cursor.fetchall() wall_ms = (time.perf_counter() - start) * 1000 cpp, py = ctx.collect() - if api != "fetchone": - assert len(rows) == 1 - row = rows[0] + assert len(rows) == 1 + row = rows[0] assert row is not None and len(row) == 1 assert type(row[0]) is type(expected) and row[0] == expected assert not cursor.messages, "Clean LOB fetch unexpectedly produced diagnostics" @@ -172,7 +157,7 @@ def lob_fetch(conn, ctx, sql_type, payload_bytes, api): wall_ms=wall_ms, cpp=cpp, py=py, - detail=f"Rows: 1; type: {sql_type}; payload bytes: {payload_bytes}; API: {api}", + detail=f"Rows: 1; type: varchar; payload bytes: {payload_bytes}; API: fetchall", ) finally: ctx.disable() @@ -190,11 +175,5 @@ def registry(): setinputsizes=(partial(legacy_insertmany, input_sizes=True), False), ) result.update((name, (partial(query, sql=sql), False)) for name, sql in QUERIES.items()) - for sql_type in ("varchar", "nvarchar", "varbinary"): - for size_kib in (64, 256): - for api in ("fetchone", "fetchmany", "fetchall"): - result[f"lob_{sql_type}_{size_kib}k_{api}"] = ( - partial(lob_fetch, sql_type=sql_type, payload_bytes=size_kib * 1024, api=api), - False, - ) + result["lob_varchar_256k_fetchall"] = (lob_fetch, False) return result diff --git a/tests/test_036_profiler_ci.py b/tests/test_036_profiler_ci.py index 24e93d440..bb4edbdc9 100644 --- a/tests/test_036_profiler_ci.py +++ b/tests/test_036_profiler_ci.py @@ -687,20 +687,16 @@ def __exit__(self, *args): def test_report_cases_match_the_executed_workload_registry(): _, workloads = controller.load_suite() assert tuple(workloads.registry()) == reporting.CASES + assert len(reporting.CASES) == 21 + assert [name for name in reporting.CASES if name.startswith("lob_")] == [ + "lob_varchar_256k_fetchall", + ] -@pytest.mark.parametrize("sql_type", ("varchar", "nvarchar", "varbinary")) -@pytest.mark.parametrize("size", (8190, 8192, 8194, 65536, 262144)) -@pytest.mark.parametrize("api", ("fetchone", "fetchmany", "fetchall")) -def test_lob_workload_validates_payload_and_times_only_fetch(sql_type, size, api, monkeypatch): - expected = ( - b"\x00x" * (size // 2) - if sql_type == "varbinary" - else "\u00e9" * (size // 2) if sql_type == "nvarchar" else "x" * size - ) +def test_lob_workload_validates_payload_and_times_only_fetch(monkeypatch): + size = 256 * 1024 + expected = "x" * size cursor = MagicMock() - cursor.fetchone.return_value = (expected,) - cursor.fetchmany.return_value = [(expected,)] cursor.fetchall.return_value = [(expected,)] cursor.messages = [] connection = MagicMock() @@ -710,18 +706,17 @@ def test_lob_workload_validates_payload_and_times_only_fetch(sql_type, size, api def enable(): cursor.execute.assert_called_once() - for method in ("fetchone", "fetchmany", "fetchall"): - getattr(cursor, method).assert_not_called() + cursor.fetchall.assert_not_called() context.enable.side_effect = enable monkeypatch.setattr(benchmark_workloads.time, "perf_counter", MagicMock(side_effect=[1, 1.1])) - result = benchmark_workloads.lob_fetch(connection, context, sql_type, size, api) + result = benchmark_workloads.lob_fetch(connection, context) assert "(MAX)" in cursor.execute.call_args.args[0] assert result["wall_ms"] == pytest.approx(100) - assert result["detail"] == f"Rows: 1; type: {sql_type}; payload bytes: {size}; API: {api}" - getattr(cursor, api).assert_called_once_with(*((1,) if api == "fetchmany" else ())) - for other in {"fetchone", "fetchmany", "fetchall"} - {api}: - getattr(cursor, other).assert_not_called() + assert result["detail"] == f"Rows: 1; type: varchar; payload bytes: {size}; API: fetchall" + cursor.fetchall.assert_called_once_with() + cursor.fetchone.assert_not_called() + cursor.fetchmany.assert_not_called() context.collect.assert_called_once() context.disable.assert_called_once() @@ -731,12 +726,12 @@ def enable(): ) def test_lob_workload_rejects_invalid_results_and_always_disables(problem): cursor = MagicMock() - cursor.fetchall.return_value = [("x" * 65536,)] + cursor.fetchall.return_value = [("x" * 262144,)] cursor.messages = [] if problem == "truncated": - cursor.fetchall.return_value = [("x" * 65535,)] + cursor.fetchall.return_value = [("x" * 262143,)] elif problem == "wrong-type": - cursor.fetchall.return_value = [(b"x" * 65536,)] + cursor.fetchall.return_value = [(b"x" * 262144,)] elif problem == "missing": cursor.fetchall.return_value = [] elif problem == "extra": @@ -750,12 +745,12 @@ def test_lob_workload_rejects_invalid_results_and_always_disables(problem): context = MagicMock() context.collect.return_value = ({}, {}) with pytest.raises(RuntimeError if problem == "error" else AssertionError): - benchmark_workloads.lob_fetch(connection, context, "varchar", 65536, "fetchall") + benchmark_workloads.lob_fetch(connection, context) context.disable.assert_called_once() def test_lob_regression_reports_native_diagnostic_counts_without_assuming_missing_is_zero(report): - name = "lob_varchar_256k_fetchone" + name = "lob_varchar_256k_fetchall" label = "ddbc::AppendDiagRecords::SQLGetDiagRec_call" for pair in report["pairs"]: for scenario in pair["candidate"]["scenarios"].values(): From 9700bd7b9c0f2d1f72be8ebe2bd9010a7c966720 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 24 Sep 2026 12:56:00 +0530 Subject: [PATCH 3/5] CHORE: Keep LOB coverage independent of report special cases Use existing generic counter reporting and keep the benchmark README change to the scenario count and a short workload description. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eng/profiler_benchmarks/README.md | 36 +++---------------------------- eng/profiler_benchmarks/report.py | 14 ------------ tests/test_036_profiler_ci.py | 23 -------------------- 3 files changed, 3 insertions(+), 70 deletions(-) diff --git a/eng/profiler_benchmarks/README.md b/eng/profiler_benchmarks/README.md index b63ec107c..ef2365adf 100644 --- a/eng/profiler_benchmarks/README.md +++ b/eng/profiler_benchmarks/README.md @@ -15,34 +15,9 @@ python -m eng.profiler_benchmarks.report profiler-results/report.json The fixed registry has 21 tasks. `--scenarios` runs a local subset, but subset reports remain incomplete and cannot produce a verdict. -## Large-value fetch coverage - -One task, `lob_varchar_256k_fetchall`, fetches one 256 KiB `VARCHAR(MAX)` value -through `fetchall()`. Unlike a million short rows, this value requires LOB -continuation calls. Unicode/binary variants, other fetch APIs, and chunk-boundary -combinations belong in functional regression coverage or targeted performance -investigations rather than multiplying routine CI tasks. - -The payload size describes SQL data bytes, not row counts or a promise about the -driver's internal chunk count. Query execution/setup and exact payload/type -validation are outside the timed fetch window. An unexpected warning or truncated -value fails the workload rather than producing a successful performance verdict. - -The existing elapsed-time thresholds remain unchanged. Available -`SQLGetDiagRec` profiler counters are supporting diagnostics, not a separate gate; -an absent counter is reported as unavailable, not zero. The old -`SQLGetAllDiagRecords` helper count is not equivalent to the number of underlying -ODBC calls. Mixed-warning preservation belongs in functional driver regressions, -not these clean-payload timings. - -Both revisions run the same new workloads, so the first comparison can include a -base that predates them. Older artifacts missing these tasks remain incomplete or -invalid; they must not produce a full-coverage verdict. - -The added task runs 24 times across routine CI: two revisions, six pairs including -warmup, and two environments. It needs no benchmark table or additional build. -Measure its incremental duration on each runner; do not infer the cost from task -count alone. +`lob_varchar_256k_fetchall` fetches one 256 KiB `VARCHAR(MAX)` value to exercise +multi-chunk streaming. Query setup and exact payload validation are outside the +timed fetch window. ## Measurement contract @@ -68,11 +43,6 @@ is published. A failed aggregate build can still publish usable profiler artifac Exact-head reports may finalize after merge; stale heads are ignored. Missing, malformed, canceled, incomplete, or invalid data remains unavailable. -The two reported environments are Linux/SQL Server combinations, not all supported -operating systems. A clean report cannot rule out a macOS- or Windows-specific -LOB slowdown. Changes to streaming/diagnostic code still need targeted release-build -measurements and warning-preservation checks on those platforms. - The report highlights consistent slowdowns and improvements using the same 20% median change, 1 ms absolute change, and 80% pair-agreement requirements. diff --git a/eng/profiler_benchmarks/report.py b/eng/profiler_benchmarks/report.py index 49888aa4f..6ac2723f2 100644 --- a/eng/profiler_benchmarks/report.py +++ b/eng/profiler_benchmarks/report.py @@ -295,20 +295,6 @@ def comparisons(report): before = [s[layer].get(label) for s in base] after = [s[layer].get(label) for s in candidate] if not all(before) or not all(after): - if name.startswith("lob_") and "SQLGetDiagRec" in label: - # Missing instrumentation is not evidence of zero driver calls. - old_calls = ( - f"{statistics.median(s['calls'] for s in before):g}" - if all(before) - else "unavailable" - ) - new_calls = ( - f"{statistics.median(s['calls'] for s in after):g}" - if all(after) - else "unavailable" - ) - changed_counts.append(f"{label} ({old_calls} -> {new_calls} calls)") - continue changed_counts.append(f"{label} (added, removed, or intermittent)") continue before_calls = statistics.median(s["calls"] for s in before) diff --git a/tests/test_036_profiler_ci.py b/tests/test_036_profiler_ci.py index bb4edbdc9..903973ef4 100644 --- a/tests/test_036_profiler_ci.py +++ b/tests/test_036_profiler_ci.py @@ -749,29 +749,6 @@ def test_lob_workload_rejects_invalid_results_and_always_disables(problem): context.disable.assert_called_once() -def test_lob_regression_reports_native_diagnostic_counts_without_assuming_missing_is_zero(report): - name = "lob_varchar_256k_fetchall" - label = "ddbc::AppendDiagRecords::SQLGetDiagRec_call" - for pair in report["pairs"]: - for scenario in pair["candidate"]["scenarios"].values(): - scenario["wall_ms"] = 10 - scenario = pair["candidate"]["scenarios"][name] - scenario["wall_ms"] = 100 - scenario["cpp"][label] = dict(calls=128, total_us=1280, min_us=10, max_us=10) - reporting.validate(report) - result = next(row for row in reporting.comparisons(report) if row["name"] == name) - assert result["status"] == "regression" - assert f"{label} (unavailable -> 128 calls)" in result["counts"] - body = reporting.render([report], "c" * 40, 42) - assert "Performance regression detected" in body - assert "256 KiB VARCHAR(MAX)" in body - for pair in report["pairs"]: - pair["candidate"]["scenarios"][name]["wall_ms"] = 10 - assert ( - next(row for row in reporting.comparisons(report) if row["name"] == name)["status"] == "ok" - ) - - def test_query_workload_executes_and_collects(monkeypatch): cursor = MagicMock() cursor.fetchall.return_value = [(1,), (2,)] From 0dcb8b9fffb73cc758cf0d352b0d72d64d6da767 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 24 Sep 2026 15:50:01 +0530 Subject: [PATCH 4/5] FIX: Isolate PR performance report concurrency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/pr-profiler-report.yml | 2 +- tests/test_036_profiler_ci.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-profiler-report.yml b/.github/workflows/pr-profiler-report.yml index 1e0130d2d..a107ba2c0 100644 --- a/.github/workflows/pr-profiler-report.yml +++ b/.github/workflows/pr-profiler-report.yml @@ -14,7 +14,7 @@ permissions: pull-requests: write concurrency: - group: profiler-report-${{ github.event.pull_request.number }} + group: profiler-report-${{ github.event.pull_request.number }}-${{ github.event_name }} cancel-in-progress: true jobs: diff --git a/tests/test_036_profiler_ci.py b/tests/test_036_profiler_ci.py index 903973ef4..05eedc4bb 100644 --- a/tests/test_036_profiler_ci.py +++ b/tests/test_036_profiler_ci.py @@ -1756,6 +1756,7 @@ def test_comment_workflow_separates_same_repo_and_fork_trust(): workflow = (ROOT / ".github/workflows/pr-profiler-report.yml").read_text(encoding="utf-8") assert "pull_request:" in workflow assert "pull_request_target:" in workflow + assert "profiler-report-${{ github.event.pull_request.number }}-${{ github.event_name }}" in workflow assert "github.event.pull_request.head.repo.full_name == github.repository" in workflow assert "github.event.pull_request.head.repo.full_name != github.repository" in workflow assert ( From edb9aab17154a3eca292bfb4f058c3b7fc9b35f6 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 24 Sep 2026 15:52:10 +0530 Subject: [PATCH 5/5] STYLE: Apply profiler workflow test formatting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_036_profiler_ci.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_036_profiler_ci.py b/tests/test_036_profiler_ci.py index 05eedc4bb..eced790d5 100644 --- a/tests/test_036_profiler_ci.py +++ b/tests/test_036_profiler_ci.py @@ -1756,7 +1756,10 @@ def test_comment_workflow_separates_same_repo_and_fork_trust(): workflow = (ROOT / ".github/workflows/pr-profiler-report.yml").read_text(encoding="utf-8") assert "pull_request:" in workflow assert "pull_request_target:" in workflow - assert "profiler-report-${{ github.event.pull_request.number }}-${{ github.event_name }}" in workflow + assert ( + "profiler-report-${{ github.event.pull_request.number }}-${{ github.event_name }}" + in workflow + ) assert "github.event.pull_request.head.repo.full_name == github.repository" in workflow assert "github.event.pull_request.head.repo.full_name != github.repository" in workflow assert (