Skip to content

Commit 09932d5

Browse files
committed
chore: address omair's feedback on timeouts, baseline logic, thresholds, and tempfiles
1 parent ff77903 commit 09932d5

3 files changed

Lines changed: 33 additions & 20 deletions

File tree

.github/workflows/import-profiler.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ permissions:
1515
jobs:
1616
import-profile:
1717
runs-on: ubuntu-latest
18+
timeout-minutes: 60
1819
steps:
1920
- name: Checkout
2021
uses: actions/checkout@v6

ci/run_single_test.sh

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -106,43 +106,48 @@ case ${TEST_TYPE} in
106106
esac
107107
;;
108108
import_profile)
109-
if [ -f setup.py ]; then
109+
if [ -f setup.py ] || [ -f pyproject.toml ]; then
110110
echo "Creating temporary virtualenv for import profile..."
111111
python3 -m venv .venv-profiler
112112
source .venv-profiler/bin/activate
113-
pip install -e .
114113

115114
PACKAGE_NAME=$(basename $(pwd))
116-
# Save the new profiler script to /tmp so we don't lose it during HEAD^1 checkout
117-
cp ../../scripts/import_profiler/profiler.py /tmp/profiler.py
118-
PROFILER_SCRIPT="/tmp/profiler.py"
115+
PROFILER_TEMP_DIR=$(mktemp -d)
116+
cp ../../scripts/import_profiler/profiler.py "${PROFILER_TEMP_DIR}/profiler.py"
117+
PROFILER_SCRIPT="${PROFILER_TEMP_DIR}/profiler.py"
118+
BASELINE_CSV="${PROFILER_TEMP_DIR}/baseline_${PACKAGE_NAME}.csv"
119119

120-
rm -f /tmp/baseline.csv
121120
if [ -n "${TARGET_BRANCH}" ]; then
122-
if git rev-parse HEAD^1 >/dev/null 2>&1; then
123-
echo "Checking out HEAD^1 for baseline..."
124-
git checkout HEAD^1
125-
if [ -f setup.py ]; then
126-
python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --csv /tmp/baseline.csv
121+
# Suppress errors if branch isn't found
122+
BASELINE_COMMIT=$(git merge-base HEAD "origin/${TARGET_BRANCH:-main}" 2>/dev/null || true)
123+
if [ -n "${BASELINE_COMMIT}" ]; then
124+
echo "Checking out baseline commit ${BASELINE_COMMIT}..."
125+
git checkout ${BASELINE_COMMIT}
126+
if [ -f setup.py ] || [ -f pyproject.toml ]; then
127+
pip install -e .
128+
python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --csv "${BASELINE_CSV}"
127129
else
128-
echo "setup.py not found on baseline. Skipping baseline generation."
130+
echo "setup.py/pyproject.toml not found on baseline. Skipping baseline generation."
129131
fi
130132
git checkout -
131133
else
132-
echo "Could not find HEAD^1. Skipping baseline generation."
134+
echo "Could not find baseline commit for origin/${TARGET_BRANCH:-main}. Skipping baseline generation."
133135
fi
134136
fi
135137

136-
if [ -f /tmp/baseline.csv ]; then
137-
python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --fail-threshold 5000 --diff-baseline /tmp/baseline.csv --diff-threshold 100
138+
pip install -e .
139+
140+
if [ -f "${BASELINE_CSV}" ]; then
141+
python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --fail-threshold 5000 --diff-baseline "${BASELINE_CSV}" --diff-threshold 100
138142
else
139143
python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --fail-threshold 5000
140144
fi
141145
retval=$?
142146
deactivate
143147
rm -rf .venv-profiler
148+
rm -rf "${PROFILER_TEMP_DIR}"
144149
else
145-
echo "Skipping import_profile as this does not appear to be a Python package (no setup.py)."
150+
echo "Skipping import_profile as this does not appear to be a Python package (no setup.py or pyproject.toml)."
146151
retval=0
147152
fi
148153
;;

scripts/import_profiler/profiler.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,11 +265,18 @@ def run_master(iterations, target_module, cpu=0, csv_path=None, clear_cache=True
265265
)
266266
final_messages.append(diff_msg)
267267

268-
if diff > diff_threshold:
269-
final_messages.append(f"FAILURE: Import time regression of {diff:.2f} ms exceeds the allowed threshold of {diff_threshold} ms.")
268+
relative_diff_threshold = 0.15 * baseline_p99
269+
if diff > diff_threshold and diff > relative_diff_threshold:
270+
final_messages.append(
271+
f"FAILURE: Import time regression of {diff:.2f} ms exceeds both the absolute threshold ({diff_threshold} ms) "
272+
f"and the relative threshold ({relative_diff_threshold:.2f} ms, 15% of baseline P99)."
273+
)
270274
exit_code = 1
271275
else:
272-
final_messages.append("SUCCESS: Import time diff is within acceptable thresholds.")
276+
if diff > diff_threshold:
277+
final_messages.append(f"SUCCESS: Import time regression of {diff:.2f} ms exceeds absolute threshold ({diff_threshold} ms) but is within relative threshold ({relative_diff_threshold:.2f} ms, 15%).")
278+
else:
279+
final_messages.append("SUCCESS: Import time diff is within acceptable thresholds.")
273280
else:
274281
final_messages.append(f"WARNING: Baseline CSV {diff_baseline} not found. Skipping diff check.")
275282

@@ -386,7 +393,7 @@ def find_module_from_package(pkg):
386393
if os.path.exists('setup.py') or os.path.exists('pyproject.toml'):
387394
pkgs = setuptools.find_namespace_packages(where='.')
388395
for p in sorted(pkgs, key=len):
389-
if p.startswith('tests'):
396+
if p in ("google", "google.cloud") or p.startswith("tests"):
390397
continue
391398
path = p.replace('.', os.sep)
392399
if os.path.isfile(os.path.join(path, '__init__.py')):

0 commit comments

Comments
 (0)