Skip to content

write/ benchmark scenario cannot fail the perf regression gate #534

Description

@Theekshna

Context

PR #512 added the write/ (executemany/parameter-array) benchmark scenario, but flagged in review (#512 (comment)... on mssql-odbc-bench/perf-lab/run-benchmarks.sh:475 and the equivalent run-benchmarks.ps1:373-380) that the regression gate treats it as apples-to-oranges:

  • The baseline leg is forced to sequential mode (looped single-row execute) because the pinned baseline driver rejects PARAMSET_SIZE > 1 with HYC00.
  • The candidate leg runs parameter_array (batched) mode.
  • compare-odbc-benchmarks.py compares candidate_seconds / base_seconds for every scenario including write/, using DEFAULT_REGRESSION_RATIO = 1.05 (5%) as the failure threshold.
  • With this PR's own published medians (1588.03 ms baseline sequential, 25.75 ms candidate batched) the ratio is ~0.0162. The candidate would have to regress roughly 65x before the gate would flag it — every other scenario is guarded at 5%.
  • Because initial_improvement = initial_ratio <= 1/1.05 is essentially always true for write/, and improvements are sorted ascending and capped to --improvement-max 3, write/ permanently occupies one of the three improvement-verification slots on every perf run (each costing a full paired re-run), crowding out genuine improvements from other scenarios.
  • The mode split is selected by driver name rather than capability (if scenario == "write" and driver == BASELINE_DRIVER_NAME: write_mode = sequential), so once the baseline is re-pinned to any commit at or after this PR (one that can run parameter arrays), the baseline leg is still forced to sequential and the comparison stays apples-to-oranges indefinitely.

Suggested options (roughly by effort, least first)

  1. Gate write/ on candidate_vs_reference_ratio (vs. the Microsoft msodbcsql driver) instead of vs. the sequential-mode baseline.
  2. Select write_mode by probing driver capability (e.g. attempt SQLSetStmtAttr(SQL_ATTR_PARAMSET_SIZE, 2) against the baseline driver) rather than matching driver name, so the split self-heals once the baseline supports parameter arrays.
  3. Exclude write/ from the improvement-verification slot allocation in confirmation_plan (.pipeline/scripts/compare-odbc-benchmarks.py) so it stops permanently consuming one of --improvement-max slots, independent of fixing the regression-gate threshold itself.

Any regression-gate rewrite needs a perf-lab run to validate before landing, since a mistake here silently disables (or falsely trips) the gate for CI.

Credit: flagged by Vahid (@Vahid-b) in PR #512 review.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions