feat(xtest): benchmark two named refs via workflow_dispatch

.github/workflows/xtest.yml

@@ -38,6 +38,31 @@
         type: boolean
         default: false
         description: "Run the SDK performance regression benchmarks (adds ~45m per SDK). Needs two builds per SDK: set the *-ref inputs to 'main latest', since a bare 'main' installs no release to use as a baseline and every cell will skip."
+      bench-baseline-ref:
+        required: false
+        type: string
+        default: ""
+        description: "Benchmark the ref named here against bench-candidate-ref, instead of the default newest-release-vs-branch-head comparison. Any ref otdf-sdk-mgr resolves: 'main', a branch, a tag, a SHA, 'refs/pull/N/head'. Requires bench-candidate-ref and a focus-sdk naming one SDK; ignores the *-ref inputs, which drive the functional matrix rather than this."
+      bench-candidate-ref:
+        required: false
+        type: string
+        default: ""
+        description: "The build under suspicion, measured against bench-baseline-ref. e.g. 'feat/DSPX-2604-createtdf-chunked'."
+      bench-payloads:
+        required: false
+        type: string
+        default: ""
+        description: "Payload sizes to benchmark, comma-separated, e.g. '1KiB,1MiB,32MiB,1GiB'. Default is 1KiB,1MiB,32MiB, at which ~86% of a go encrypt is fixed startup cost -- so the 1.15x gate is wider than the whole payload-dependent part and no throughput change can fail a cell. Add 1GiB to actually gate throughput; it needs a bench-budget-seconds to match and ~5 GiB of runner disk."
+      bench-budget-seconds:
+        required: false
+        type: string
+        default: ""
+        description: "Wall-clock allowance shared by every benchmark cell (default 1500). Manual runs are not on the nightly's schedule, so this is the knob to raise when adding payload sizes -- each one adds an encrypt and a decrypt cell, and the budget is divided evenly as cells start."
+      bench-max-rounds:
+        required: false
+        type: string
+        default: ""
+        description: "Hard cap on paired rounds per cell (default 60). Raise it together with the budget: at the default, cells routinely stop on max_rounds with budget left over, and every unspent round is interval width that could have been bought."
   workflow_call:
     inputs:
       platform-ref:
@@ -68,6 +93,26 @@
         required: false
         type: boolean
         default: false
+      bench-baseline-ref:
+        required: false
+        type: string
+        default: ""
+      bench-candidate-ref:
+        required: false
+        type: string
+        default: ""
+      bench-payloads:
+        required: false
+        type: string
+        default: ""
+      bench-budget-seconds:
+        required: false
+        type: string
+        default: ""
+      bench-max-rounds:
+        required: false
+        type: string
+        default: ""
   schedule:
     - cron: "30 6 * * *" # 0630 UTC
     - cron: "0 5 * * 1,3" # 500 UTC (Monday, Wednesday)
@@ -88,6 +133,7 @@
       platform-tag-list: ${{ steps.version-info.outputs.platform-tag-list }}
       heads: ${{ steps.version-info.outputs.platform-heads }}
       default-tags: ${{ steps.version-info.outputs.default-tags }}
+      bench-sdks: ${{ steps.bench-inputs.outputs.sdks }}
       go: ${{ steps.version-info.outputs.go-version-info }}
       java: ${{ steps.version-info.outputs.java-version-info }}
       js: ${{ steps.version-info.outputs.js-version-info }}
@@ -107,6 +153,45 @@
             echo "Invalid focus-sdk input: ${FOCUS_SDK_INPUT}. Must be one of: all, go, java, js." >> "$GITHUB_STEP_SUMMARY"
             exit 1
           fi
+      # Decided here rather than in the bench job because a matrix cannot be
+      # narrowed from inside the job it belongs to: a bad combination would
+      # already have spun up three runners for 45 minutes each.
+      - name: Validate benchmark inputs and pick the bench matrix
+        id: bench-inputs
+        env:
+          FOCUS_SDK: ${{ inputs.focus-sdk || 'all' }}
+          BASELINE_REF: ${{ inputs.bench-baseline-ref }}
+          CANDIDATE_REF: ${{ inputs.bench-candidate-ref }}
+          BUDGET_SECONDS: ${{ inputs.bench-budget-seconds }}
+          MAX_ROUNDS: ${{ inputs.bench-max-rounds }}
+        run: |-
+          # Only the numeric inputs are checked here. bench-payloads has a
+          # grammar, and a second copy of it in bash would drift from the one
+          # pytest enforces and start rejecting runs that would have worked;
+          # the bench job validates it with the real parser instead.
+          for pair in "bench-budget-seconds:$BUDGET_SECONDS" "bench-max-rounds:$MAX_ROUNDS"; do
+            name=${pair%%:*}
+            value=${pair#*:}
+            if [[ -n "$value" && ! "$value" =~ ^[1-9][0-9]*$ ]]; then
+              echo "::error::${name} must be a positive whole number, got '${value}'."
+              exit 1
+            fi
+          done
+          if [[ -n "$BASELINE_REF" && -z "$CANDIDATE_REF" ]] \
+             || [[ -z "$BASELINE_REF" && -n "$CANDIDATE_REF" ]]; then
+            echo "::error::bench-baseline-ref and bench-candidate-ref must be set together; a comparison needs both arms named."
+            exit 1
+          fi
+          if [[ -n "$CANDIDATE_REF" && "$FOCUS_SDK" == "all" ]]; then
+            echo "::error::bench-baseline-ref/bench-candidate-ref name refs of one SDK, so focus-sdk must be go, java, or js -- not 'all'."
+            exit 1
+          fi
+          if [[ "$FOCUS_SDK" == "all" ]]; then
+            echo 'sdks=["go","java","js"]' >> "$GITHUB_OUTPUT"
+          else
+            echo "sdks=[\"${FOCUS_SDK}\"]" >> "$GITHUB_OUTPUT"
+          fi
+
       - name: Default Versions depend on context
         id: default-tags
         run: |-
@@ -769,7 +854,14 @@
   # Never runs on pull requests: 30 minutes of serial measurement is too slow
   # for a PR gate, and a PR runner is the noisiest place to measure.
   bench:
-    timeout-minutes: 45
+    # Has to cover setup plus the whole of bench-budget-seconds, and setup is
+    # not a small constant: a warm Go module cache builds both arms in ~3
+    # minutes, a cold one took 19. At 45 this job could not even finish its
+    # own default 1500s budget after a cold start -- it would be killed
+    # mid-measurement, which loses the report entirely rather than reporting
+    # fewer rounds. The budget is the knob that bounds the run; this is only
+    # the backstop for a hung one.
+    timeout-minutes: 90
     runs-on: ubuntu-latest
     needs: resolve-versions
     # Nightly cron only, not the Mon/Wed or weekly ones: three runs a week of
@@ -783,17 +875,42 @@
       packages: read
     strategy:
       # One runner per SDK. Two SDKs on one runner would contend for the very
-      # CPU being measured.
+      # CPU being measured. Narrowed by focus-sdk, so investigating one SDK
+      # does not spend 45 minutes measuring the two nobody asked about.
       fail-fast: false
       matrix:
-        sdk: [go, java, js]
+        sdk: ${{ fromJSON(needs.resolve-versions.outputs.bench-sdks) }}
     steps:
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
         with:
           repository: opentdf/tests
           path: otdftests
           persist-credentials: false
 
+      # Before the platform, which takes ~15 minutes to come up: a typo'd size
+      # is worth catching in the first thirty seconds. This calls the harness's
+      # own parser rather than reimplementing the grammar in bash -- perf.cells
+      # imports nothing outside the standard library, so a bare python3 can
+      # read it, and a bash copy that drifted would start refusing specs the
+      # run itself would have accepted.
+      - name: Validate benchmark payload sizes
+        if: inputs.bench-payloads != ''
+        working-directory: otdftests/xtest
+        env:
+          BENCH_PAYLOADS: ${{ inputs.bench-payloads }}
+        run: |-
+          python3 - "$BENCH_PAYLOADS" <<'PY'
+          import sys
+
+          from perf.cells import parse_payloads
+
+          try:
+              sizes = parse_payloads(sys.argv[1])
+          except ValueError as e:
+              raise SystemExit(f"::error::invalid bench-payloads: {e}")
+          print("payload sizes:", ", ".join(p.label for p in sizes))
+          PY
+
       - name: load extra keys from file
         id: load-extra-keys
         run: |-
@@ -859,16 +976,57 @@
           PLATFORM_DIR: ${{ steps.run-platform.outputs.platform-working-dir }}
 
       ######## INSTALL BOTH ARMS OF THE COMPARISON #############
+      # Two named refs instead of the default release-vs-branch pair. Resolved
+      # here rather than in resolve-versions because the *-ref inputs there
+      # drive the functional matrix, and a benchmark wants to name its two
+      # arms without also changing what the rest of the workflow tests.
+      #
+      # Baseline first: the tag order becomes configure-sdk's `heads` output,
+      # and conftest.py takes heads[0] as the otdfctl that provisions
+      # attributes and the KAS registry. That provisioning is not measured,
+      # and it should be the same build for both arms.
+      - name: Resolve the two benchmark arms
+        id: bench-arms
+        if: inputs.bench-candidate-ref != ''
+        working-directory: otdftests/otdf-sdk-mgr
+        env:
+          SDK: ${{ matrix.sdk }}
+          BASELINE_REF: ${{ inputs.bench-baseline-ref }}
+          CANDIDATE_REF: ${{ inputs.bench-candidate-ref }}
+        run: |-
+          info=$(uv run --project . otdf-sdk-mgr versions resolve \
+                   "$SDK" "$BASELINE_REF" "$CANDIDATE_REF")
+          jq . <<<"$info"
+          err=$(jq -r '[.[] | select(.err != null) | .err] | join("; ")' <<<"$info")
+          if [[ -n "$err" ]]; then
+            echo "::error::Could not resolve benchmark arms: $err"
+            exit 1
+          fi
+          # `versions resolve` drops a ref whose SHA it has already seen, so
+          # two names for one commit come back as a single entry. Left alone
+          # that installs one build, fails arm selection in every cell, and
+          # spends the runner's 45 minutes arriving at NOTHING MEASURED.
+          if [[ "$(jq 'length' <<<"$info")" -ne 2 ]]; then
+            echo "::error::${BASELINE_REF} and ${CANDIDATE_REF} resolve to the same commit -- nothing to compare."
+            exit 1
+          fi
+          {
+            echo "version-info=$(jq -c . <<<"$info")"
+            echo "baseline-spec=${SDK}@$(jq -r '.[0].tag' <<<"$info")"
+            echo "candidate-spec=${SDK}@$(jq -r '.[1].tag' <<<"$info")"
+          } >> "$GITHUB_OUTPUT"
+
       # The whole design rests on this step laying down two builds side by
-      # side under sdk/<sdk>/dist/: the branch head (candidate) and the
-      # newest release (baseline). Arm selection picks them up from there.
+      # side under sdk/<sdk>/dist/: by default the branch head (candidate) and
+      # the newest release (baseline), or the two refs resolved above. Arm
+      # selection picks them up from there.
       - name: Configure ${{ matrix.sdk }} sdk
         id: configure-sdk
         uses: ./otdftests/xtest/setup-cli-tool
         with:
           path: otdftests/xtest/sdk
           sdk: ${{ matrix.sdk }}
-          version-info: "${{ needs.resolve-versions.outputs[matrix.sdk] }}"
+          version-info: "${{ steps.bench-arms.outputs.version-info || needs.resolve-versions.outputs[matrix.sdk] }}"
           platform-otdfctl-dir: ${{ steps.platform-otdfctl.outputs.dir }}
           platform-otdfctl-sha: ${{ steps.platform-otdfctl.outputs.sha }}
 
@@ -929,7 +1087,7 @@
             fi
           done
         env:
-          java_version_info: ${{ needs.resolve-versions.outputs.java }}
+          java_version_info: ${{ steps.bench-arms.outputs.version-info || needs.resolve-versions.outputs.java }}
           platform_ref: ${{ fromJSON(needs.resolve-versions.outputs.platform-tag-to-sha)['main'] }}
 
       - name: Build the ${{ matrix.sdk }} cli
@@ -962,17 +1120,37 @@
       - name: Run performance benchmarks
         id: bench
         run: |-
+          # Empty unless the two arms were named explicitly, in which case
+          # arm selection must not fall back to "newest release vs branch
+          # head": neither named ref need be a release, and with two branch
+          # builds installed the default would pick the wrong pair or none.
+          arms=()
+          if [[ -n "$BENCH_BASELINE_SPEC" ]]; then
+            arms=(--bench-baseline "$BENCH_BASELINE_SPEC"
+                  --bench-candidate "$BENCH_CANDIDATE_SPEC")
+          fi
           uv run --frozen --no-build pytest -ra -v \
             --bench \
             --sdks "$BENCH_SDK" \
-            --bench-budget-seconds 1500 \
+            "${arms[@]}" \
+            --bench-payloads "$BENCH_PAYLOADS" \
+            --bench-budget-seconds "$BENCH_BUDGET_SECONDS" \
+            --bench-max-rounds "$BENCH_MAX_ROUNDS" \
             --bench-out test-results/benchmarks \
             --html "test-results/bench-${BENCH_SDK}.html" \
             --self-contained-html \
             test_benchmarks.py
         working-directory: otdftests/xtest
         env:
           BENCH_SDK: ${{ matrix.sdk }}
+          BENCH_BASELINE_SPEC: ${{ steps.bench-arms.outputs.baseline-spec }}
+          BENCH_CANDIDATE_SPEC: ${{ steps.bench-arms.outputs.candidate-spec }}
+          # Fallbacks rather than input defaults: the scheduled nightly
+          # supplies no inputs at all, so `inputs.*` is empty there and these
+          # are what keeps its matrix and budget where they have always been.
+          BENCH_PAYLOADS: ${{ inputs.bench-payloads || '1KiB,1MiB,32MiB' }}
+          BENCH_BUDGET_SECONDS: ${{ inputs.bench-budget-seconds || '1500' }}
+          BENCH_MAX_ROUNDS: ${{ inputs.bench-max-rounds || '60' }}
           PLATFORM_DIR: "../../${{ steps.run-platform.outputs.platform-working-dir }}"
           SCHEMA_FILE: "manifest.schema.json"
           PLATFORM_TAG: main

otdf-sdk-mgr/src/otdf_sdk_mgr/resolve.py

@@ -300,7 +300,14 @@ def _resolve_against(
                 "alias": version,
                 "head": True,
                 "sha": sha,
-                "tag": version,
+                # Flattened the same way _classify_sha_match flattens a branch
+                # it reached by SHA: the tag becomes a single dist/<tag>/ and
+                # src/<tag>/ path component. A slash here nests those
+                # directories, and every consumer walks them one level deep --
+                # xtest's all_versions_of() lists dist/*/ and the go Makefile
+                # finds src/*/, so "feat/x" is discovered as a "feat" build
+                # with no cli.sh in it.
+                "tag": version.replace("/", "--"),
             }
 
         if infix and version.startswith(f"{infix}/"):

otdf-sdk-mgr/tests/test_resolve.py

@@ -76,7 +76,24 @@ def test_refs_heads_non_main_branch(self):
             result = resolve("js", "refs/heads/release/sdk-v0.17", None)
         assert is_resolve_success(result)
         assert "head" in result and result["head"] is True
-        assert result["tag"] == "release/sdk-v0.17"
+        assert result["tag"] == "release--sdk-v0.17"
+        assert result["sha"] == SHA40
+
+    def test_branch_by_name_flattens_slashes(self):
+        # Same flattening the SHA path applies, and for the same reason: the
+        # tag is one path component under dist/ and src/. Reached by name
+        # rather than by SHA, which is the shape a workflow_dispatch input
+        # arrives in.
+        ls = make_ls_remote(
+            (SHA40, "refs/heads/feat/DSPX-2604-createtdf-chunked"),
+            ("d" * 40, "refs/heads/main"),
+        )
+        with patch_git(ls):
+            result = resolve("go", "feat/DSPX-2604-createtdf-chunked", None)
+        assert is_resolve_success(result)
+        assert result.get("head") is True
+        assert result["tag"] == "feat--DSPX-2604-createtdf-chunked"
+        assert result["alias"] == "feat/DSPX-2604-createtdf-chunked"
         assert result["sha"] == SHA40
 
 

xtest/conftest.py

@@ -23,9 +23,10 @@
 import pytest
 
 import tdfs
+from fixtures.bench import payloads_from_options
 from otdfctl import OpentdfCommandLineTool
 from perf import report, stats
-from perf.cells import cells_for
+from perf.cells import DEFAULT_PAYLOAD_SPEC, cells_for
 
 logging.basicConfig(level=os.environ.get("LOGLEVEL", "DEBUG"))
 
@@ -181,6 +182,16 @@ def _add_benchmark_options(parser: pytest.Parser):
         help="build under test, e.g. go@main; defaults to the installed "
         "unreleased build of each sdk",
     )
+    group.addoption(
+        "--bench-payloads",
+        default=DEFAULT_PAYLOAD_SPEC,
+        help="comma-separated payload sizes to measure, e.g. "
+        "'1KiB,1MiB,32MiB,1GiB' (default: %(default)s). Sizes above the "
+        "default are opt-in because they are what a throughput gate actually "
+        "needs and what a nightly cannot afford: each one adds two cells, and "
+        "a run holds roughly twice the total plus the largest twice over on "
+        "disk",
+    )
     group.addoption(
         "--bench-threshold",
         type=float,
@@ -342,7 +353,7 @@ def _parametrize_bench_cells(metafunc: pytest.Metafunc):
         typing.get_args(tdfs.sdk_type)
     )
     names = list(dict.fromkeys(s.split("@", 1)[0] for s in str(specs).split()))
-    cells = cells_for(names)
+    cells = cells_for(names, payloads_from_options(metafunc.config))
     metafunc.config.stash[report.CELLS_KEY] = cells
     metafunc.parametrize("bench_cell", cells, ids=[c.id for c in cells])