Merge pull request #724 from dolthub/bench/textpk-extra

sysbench: split TEXT PK into a separate Benchmark Extra workflow

Author: timsehn

.github/workflows/benchmark-extra.yml

@@ -0,0 +1,94 @@
+name: Benchmark Extra
+
+# Companion to Benchmark. Runs the TEXT PK variant of the sysbench suite
+# (test/sysbench_compare_textpk.sh) — every workload against tables with
+# a 32-char hex TEXT PRIMARY KEY. Reporting only; no ceiling check until
+# the non-INTKEY perf work brings ratios in.
+
+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]
+
+permissions:
+  contents: read
+  issues: write
+  pull-requests: write
+
+jobs:
+  benchmark-extra:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+
+    steps:
+    - uses: actions/checkout@v4
+
+    - name: Install dependencies
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y tcl-dev build-essential zlib1g-dev
+
+    - name: Configure
+      run: |
+        mkdir -p build && cd build
+        TCL_CONFIG=$(find /usr -name tclConfig.sh 2>/dev/null | head -1)
+        if [ -n "$TCL_CONFIG" ]; then
+          ../configure --with-tcl=$(dirname $TCL_CONFIG)
+        else
+          ../configure
+        fi
+
+    - name: Build doltlite and stock SQLite
+      run: |
+        cd build
+        make doltlite
+        make DOLTLITE_PROLLY=0 sqlite3
+
+    - name: Run benchmark
+      run: |
+        cd build
+        bash ../test/sysbench_compare_textpk.sh | tee /tmp/bench_textpk_results.md
+
+    - name: Comment PR with results
+      continue-on-error: true
+      uses: actions/github-script@v7
+      with:
+        script: |
+          // Read from file so the multi-line markdown (and any backticks /
+          // template-literal-unsafe chars in it) never has to round-trip
+          // through YAML scalars or JS template-literal parsing.
+          const fs = require('fs');
+          const body = fs.readFileSync('/tmp/bench_textpk_results.md', 'utf8');
+
+          if (!context.issue.number) {
+            console.log('No PR context (push event) — skipping comment');
+            return;
+          }
+
+          // Find existing benchmark-extra comment.
+          const comments = await github.rest.issues.listComments({
+            owner: context.repo.owner,
+            repo: context.repo.repo,
+            issue_number: context.issue.number,
+          });
+          const botComment = comments.data.find(c =>
+            c.body.includes('<!-- benchmark:textpk -->') ||
+            c.body.includes('## Sysbench-Style Benchmark (TEXT PK): Doltlite vs SQLite')
+          );
+
+          if (botComment) {
+            await github.rest.issues.updateComment({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              comment_id: botComment.id,
+              body: body,
+            });
+          } else {
+            await github.rest.issues.createComment({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issue_number: context.issue.number,
+              body: body,
+            });
+          }

.github/workflows/benchmark.yml

@@ -61,14 +61,15 @@ jobs:
             return;
           }
 
-          // Find existing benchmark comment
+          // Find existing classic benchmark comment
           const comments = await github.rest.issues.listComments({
             owner: context.repo.owner,
             repo: context.repo.repo,
             issue_number: context.issue.number,
           });
           const botComment = comments.data.find(c =>
-            c.body.includes('Sysbench-Style Benchmark')
+            c.body.includes('<!-- benchmark:classic -->') ||
+            c.body.includes('## Sysbench-Style Benchmark: Doltlite vs SQLite')
           );
 
           if (botComment) {

test/sysbench_compare.sh

@@ -32,13 +32,6 @@ import random, string, os
 
 random.seed($SEED)
 R = $ROWS
-# TEXT-PK section uses smaller row/op counts so the bench job stays
-# within CI's 15-minute limit. Each non-INTKEY insert on doltlite
-# currently goes through a per-statement flush whose cost scales with
-# tree size, so full-scale R blows the budget in the prepare phase
-# alone. Smaller TPR/TPN still surfaces the regression.
-TPR = max(R // 10, 100)
-TPN = max(R // 10, 100)
 d = '$TMPDIR'
 
 def rint(a, b):
@@ -96,25 +89,6 @@ def prep_with_types(f):
     write_prepare(f)
     write_prepare_types(f)
 
-def write_prepare_textpk(f):
-    # Same shape as sbtest1, but PK is a 32-char hex string (UUID-shaped).
-    # Lights up the non-INTKEY mutmap-flush path (issue #718).
-    # Sized smaller than the INTKEY suite (TPR rows vs R) because each
-    # non-INTKEY insert on doltlite goes through a per-statement flush
-    # whose cost scales with tree size, and the section's job is to
-    # surface the regression — full-scale R would push the bench job
-    # past CI's 15-minute limit. Keep the workload N relative to TPR
-    # so per-test wall time stays comparable to the INTKEY tests.
-    f.write("CREATE TABLE sbtest_textpk(id TEXT PRIMARY KEY, k INTEGER NOT NULL DEFAULT 0, c TEXT NOT NULL DEFAULT '', pad TEXT NOT NULL DEFAULT '');\n")
-    f.write("CREATE INDEX k_idx_textpk ON sbtest_textpk(k);\n")
-    f.write("BEGIN;\n")
-    for i in range(1, TPR+1):
-        f.write(f"INSERT INTO sbtest_textpk VALUES('{i:032x}',{rint(1,TPR)},'{rstr(60)}','{rstr(30)}');\n")
-    f.write("COMMIT;\n")
-
-def prep_textpk(f):
-    write_prepare_textpk(f)
-
 # --- Tests ---
 
 def w_bulk_insert(f):
@@ -363,41 +337,6 @@ def w_read_write_autocommit(f):
         f.write(f"DELETE FROM sbtest1 WHERE id={id};\n")
         f.write(f"INSERT OR REPLACE INTO sbtest1 VALUES({id},{rint(1,R)},'{rstr(60)}','{rstr(30)}');\n")
 
-# TEXT-PK writes: same workload shapes as the INTEGER-PK writes, but the
-# PK is a 32-char hex string. Issue #718: non-INTKEY tables route through
-# mergeWalk on every flush, which is 10-1000x slower than streamingMerge.
-# Reporting only (not gated) until the fix lands.
-def w_update_index_textpk(f):
-    f.write("BEGIN;\n")
-    for _ in range(TPN):
-        f.write(f"UPDATE sbtest_textpk SET k={rint(1,TPR)} WHERE id='{rint(1,TPR):032x}';\n")
-    f.write("COMMIT;\n")
-
-def w_update_non_index_textpk(f):
-    f.write("BEGIN;\n")
-    for _ in range(TPN):
-        f.write(f"UPDATE sbtest_textpk SET c='{rstr(60)}' WHERE id='{rint(1,TPR):032x}';\n")
-    f.write("COMMIT;\n")
-
-def w_oltp_insert_textpk(f):
-    f.write("BEGIN;\n")
-    for i in range(TPR+1, TPR+TPN+1):
-        f.write(f"INSERT INTO sbtest_textpk VALUES('{i:032x}',{rint(1,TPR)},'{rstr(60)}','{rstr(30)}');\n")
-    f.write("COMMIT;\n")
-
-def w_delete_insert_textpk(f):
-    f.write("BEGIN;\n")
-    for _ in range(TPN):
-        i = rint(1, TPR)
-        f.write(f"DELETE FROM sbtest_textpk WHERE id='{i:032x}';\n")
-        f.write(f"INSERT OR REPLACE INTO sbtest_textpk VALUES('{i:032x}',{rint(1,TPR)},'{rstr(60)}','{rstr(30)}');\n")
-    f.write("COMMIT;\n")
-
-make_test("oltp_update_index_textpk",     prep_textpk, w_update_index_textpk)
-make_test("oltp_update_non_index_textpk", prep_textpk, w_update_non_index_textpk)
-make_test("oltp_insert_textpk",           prep_textpk, w_oltp_insert_textpk)
-make_test("oltp_delete_insert_textpk",    prep_textpk, w_delete_insert_textpk)
-
 make_test("oltp_bulk_insert_ac",      prep_main, w_bulk_insert_autocommit)
 make_test("oltp_insert_ac",           prep_main, w_oltp_insert_autocommit)
 make_test("oltp_update_index_ac",     prep_main, w_update_index_autocommit)
@@ -442,9 +381,8 @@ else:
 }
 
 bench_runs_for_test() {
-  # BENCH_RUNS=1 for fast local iteration; default 7 for stable median
-  # while keeping the bench job under CI's 15-minute limit.
-  echo "${BENCH_RUNS:-7}"
+  # BENCH_RUNS=1 for fast local iteration; default 11 for stable median.
+  echo "${BENCH_RUNS:-11}"
 }
 
 median_us() {
@@ -479,7 +417,6 @@ run_bench_stable() {
 READ_TESTS="oltp_point_select oltp_range_select oltp_sum_range oltp_order_range oltp_distinct_range oltp_index_scan select_random_points select_random_ranges covering_index_scan groupby_scan index_join index_join_scan types_table_scan table_scan oltp_read_only"
 WRITE_TESTS="oltp_bulk_insert oltp_insert oltp_update_index oltp_update_non_index oltp_delete_insert oltp_write_only types_delete_insert oltp_read_write"
 WRITE_TESTS_AC="oltp_bulk_insert_ac oltp_insert_ac oltp_update_index_ac oltp_update_non_index_ac oltp_delete_insert_ac oltp_write_only_ac types_delete_insert_ac oltp_read_write_ac"
-WRITE_TESTS_TEXTPK="oltp_update_index_textpk oltp_update_non_index_textpk oltp_insert_textpk oltp_delete_insert_textpk"
 
 # ============================================================
 # Output markdown table
@@ -515,6 +452,7 @@ run_section() {
   echo "| Average |  |  | ${avg_ratio} |"
 }
 
+echo "<!-- benchmark:classic -->"
 echo "## Sysbench-Style Benchmark: Doltlite vs SQLite"
 echo ""
 echo "### In-Memory"
@@ -555,16 +493,6 @@ echo "#### Writes"
 echo ""
 run_section "$WRITE_TESTS_AC" "/tmp/bench_file" "/tmp/bench_file"
 
-echo ""
-echo "### File-Backed (TEXT PK writes)"
-echo ""
-echo "_Same workload shapes as the File-Backed Writes section, but on a_"
-echo "_table with a 32-char hex \`TEXT PRIMARY KEY\`. Surfaces the non-INTKEY_"
-echo "_mutmap-flush path (issue #718). Reporting only — not gated by the_"
-echo "_ceiling check below until the fix lands._"
-echo ""
-run_section "$WRITE_TESTS_TEXTPK" "/tmp/bench_file" "/tmp/bench_file"
-
 echo ""
 echo "_${ROWS} rows, single CLI invocation per test, workload-only timing via SQL timestamps._"