Skip to content

Commit 00a486e

Browse files
committed
minor: modified the comments and perf results parsing
1 parent b8e9754 commit 00a486e

3 files changed

Lines changed: 64 additions & 129 deletions

File tree

.github/workflows/test-runner.yml

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,11 @@ jobs:
114114
concurrency:
115115
group: ${{ github.workflow }}-${{ github.ref }}-benchmark-existing
116116
cancel-in-progress: true
117-
# Runs *after* windows-benchmark-bq rather than alongside it. Both shards
118-
# benchmark against the same BigQuery project, so running them concurrently
119-
# made each one's numbers depend on the other's API load -- and when that
120-
# tripped rate limits, retry backoff turned the contention into
121-
# multi-second steps. The dependency deliberately does not require the
122-
# BqDriver shard to succeed: '!cancelled()' plus the windows-cmake guard
123-
# preserves the previous run conditions, so the existing-driver baseline is
124-
# still produced when that shard fails or is skipped.
125117
if: |
126118
!cancelled() &&
127119
github.event_name == 'workflow_dispatch' &&
128120
inputs.run_benchmark_existing == true
129-
needs: [pre-flight, windows-benchmark-bq]
121+
needs: [pre-flight]
130122
uses: ./.github/workflows/windows-benchmark.yml
131123
with:
132124
checkout-ref: ${{ needs.pre-flight.outputs.checkout-sha }}
@@ -176,17 +168,18 @@ jobs:
176168
import os
177169
import re
178170
171+
def clean_test_name(name):
172+
# GTest names follow [Instantiation/]TestSuite.TestCase[/Param]
173+
# Extract everything after '.' to generically remove TestSuite/Instantiation prefix
174+
if '.' in name:
175+
name = name.split('.', 1)[1]
176+
# Strip legacy parameter suffix if comparing against older baselines
177+
name = re.sub(r'/(?:With|Without)HTAPI$', '', name)
178+
return name
179+
179180
def parse_gtest_output(filepath):
180-
# The suite is run several times (--gtest_repeat), so each test
181+
# The suite is run several times across iterations, so each test
181182
# appears once per repetition. Take the median of its timings.
182-
#
183-
# These benchmarks talk to a live BigQuery service and fan list
184-
# calls out concurrently, so a single run samples the latency
185-
# tail and moved tens of percent between runs for no code
186-
# reason. The median rejects a single outlier repetition,
187-
# including the cold-cache first one. Files with only one run per
188-
# test (a --gtest_repeat=1 run, or a baseline recorded before
189-
# this change) still work: the median of one sample is itself.
190183
samples = {}
191184
if not os.path.exists(filepath):
192185
return {}
@@ -197,9 +190,10 @@ jobs:
197190
for line in f:
198191
match = pattern.search(line)
199192
if match:
193+
test_name = clean_test_name(match.group(1))
200194
ms = parse_time_to_ms(match.group(2))
201195
if ms is not None:
202-
samples.setdefault(match.group(1), []).append(ms)
196+
samples.setdefault(test_name, []).append(ms)
203197
except Exception as e:
204198
print(f'Error reading {filepath}: {e}')
205199
@@ -245,11 +239,6 @@ jobs:
245239
else:
246240
return ' (0%)'
247241
248-
def clean_test_name(name):
249-
name = name.replace('HTAPIVariations/CatalogPerformanceHtapiParamTest.', '')
250-
name = name.replace('DataFetchPerformanceParamTest.', '')
251-
return name
252-
253242
existing_data = parse_gtest_output('./benchmark_results/current_core.txt')
254243
current_bq_data = parse_gtest_output('./benchmark_results/current_bq.txt')
255244
main_bq_data = parse_gtest_output('./benchmark_results/main_bq.txt')
@@ -259,8 +248,6 @@ jobs:
259248
260249
rows = []
261250
for test in sorted_tests:
262-
cleaned_name = clean_test_name(test)
263-
264251
existing_raw = existing_data.get(test, 'N/A')
265252
cur_bq_raw = current_bq_data.get(test, 'N/A')
266253
main_bq_raw = main_bq_data.get(test, 'N/A')
@@ -275,7 +262,7 @@ jobs:
275262
cur_bq_val = f'{cur_bq_raw}{cur_bq_pct}'
276263
main_bq_val = f'{main_bq_raw}{main_bq_pct}'
277264
278-
rows.append((cleaned_name, existing_raw, cur_bq_val, main_bq_val))
265+
rows.append((test, existing_raw, cur_bq_val, main_bq_val))
279266
280267
# Define headers
281268
h1 = 'Test Case (HTAPI ON/OFF)'

.github/workflows/windows-benchmark.yml

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ on:
1515
benchmark_iterations:
1616
required: false
1717
description: >-
18-
How many times to run the whole suite (gtest --gtest_repeat). The
19-
results table reports the median per test, which rejects a single
20-
outlier run. Total runtime scales linearly with this.
18+
How many times to run the whole suite. The results table reports
19+
the median per test, which rejects a single outlier run. Total
20+
runtime scales linearly with this.
2121
type: string
2222
default: "3"
2323
secrets:
@@ -163,18 +163,9 @@ jobs:
163163
echo "Running performance benchmark executable against $BUILD_SHARD..."
164164
echo " repeats=${BENCHMARK_ITERATIONS}"
165165
set +e
166-
# Run the suite BENCHMARK_ITERATIONS times and let the results parser
167-
# take the median of each test's timings. A single run is dominated by
168-
# BigQuery/service latency variance, so one sample per test moved tens
169-
# of percent between runs for no code reason.
170-
#
171-
# Deliberately separate processes rather than --gtest_repeat: repeating
172-
# in-process re-allocates and frees SQL_HANDLE_ENV once per test, so
173-
# the Driver Manager loads and unloads the driver DLL on every test.
174-
# Tripling that churn crashed the Simba driver mid-run (abort, exit 3).
175-
# A fresh process per repetition keeps that count identical to a
176-
# single-shot run, and an iteration that dies still leaves the other
177-
# iterations' timings in the results file.
166+
# Run the suite in a separate process for each iteration to avoid DLL
167+
# reload churn across tests. The results parser takes the median of
168+
# timings across iterations.
178169
: > "$RESULTS_FILE"
179170
TEST_EXIT_CODE=0
180171
for i in $(seq 1 "${BENCHMARK_ITERATIONS}"); do

0 commit comments

Comments
 (0)