-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ci: add import profiler check across monorepo #17657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6d60f59
da118aa
5eab435
8ccfb28
a0c075f
a7d2363
68a0bdf
42dd64a
a6491b5
9472ab6
705b188
885de7f
a2bd6ec
a77dc18
8a2d75b
9732c4e
df592fe
b87f403
483e44b
943133d
8aff62c
ff77903
09932d5
c05ad7d
80ed579
e06bd2d
ee1645b
6a972d8
1c13c8c
85f4a4f
b100f3a
c6a0a71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| name: import-profiler | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: | ||
| - main | ||
| - preview | ||
| # Trigger workflow on GitHub merge queue events | ||
| merge_group: | ||
| types: [checks_requested] | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| import-profile: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 60 | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 2 | ||
| - name: Setup Python | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: "3.15" | ||
| allow-prereleases: true | ||
| - name: Upgrade pip, setuptools, and wheel | ||
| run: | | ||
| python -m pip install --upgrade setuptools pip wheel | ||
| - name: Run import profiler | ||
| env: | ||
| BUILD_TYPE: presubmit | ||
| TARGET_BRANCH: ${{ github.base_ref || github.event.merge_group.base_ref }} | ||
| TEST_TYPE: import_profile | ||
| PY_VERSION: "3.15" | ||
| run: | | ||
| ci/run_conditional_tests.sh | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,61 @@ case ${TEST_TYPE} in | |
| ;; | ||
| esac | ||
| ;; | ||
| import_profile) | ||
| if [ -f setup.py ] || [ -f pyproject.toml ]; then | ||
| echo "Creating temporary virtualenv for import profile..." | ||
| python3 -m venv .venv-profiler | ||
| source .venv-profiler/bin/activate | ||
|
|
||
| PACKAGE_NAME=$(basename $(pwd)) | ||
| PROFILER_TEMP_DIR=$(mktemp -d) | ||
| cp ../../scripts/import_profiler/profiler.py "${PROFILER_TEMP_DIR}/profiler.py" | ||
| PROFILER_SCRIPT="${PROFILER_TEMP_DIR}/profiler.py" | ||
| BASELINE_CSV="${PROFILER_TEMP_DIR}/baseline_${PACKAGE_NAME}.csv" | ||
|
|
||
| if [ -n "${TARGET_BRANCH}" ]; then | ||
| # Try upstream first (for forks), then origin | ||
| BASELINE_COMMIT=$(git merge-base HEAD "upstream/${TARGET_BRANCH}" 2>/dev/null || \ | ||
| git merge-base HEAD "origin/${TARGET_BRANCH}" 2>/dev/null || \ | ||
| git merge-base HEAD "${TARGET_BRANCH}" 2>/dev/null || true) | ||
| if [ -n "${BASELINE_COMMIT}" ]; then | ||
| echo "Checking out baseline commit ${BASELINE_COMMIT} in a temporary worktree..." | ||
| REPO_PREFIX=$(git rev-parse --show-prefix) | ||
| WORKTREE_DIR=$(mktemp -d) | ||
| rmdir "${WORKTREE_DIR}" | ||
| if git worktree add "${WORKTREE_DIR}" "${BASELINE_COMMIT}" 2>/dev/null; then | ||
| ( | ||
| cd "${WORKTREE_DIR}/${REPO_PREFIX}" | ||
| if [ -f setup.py ] || [ -f pyproject.toml ]; then | ||
| pip install -e . | ||
| python "${PROFILER_SCRIPT}" --package "${PACKAGE_NAME}" --iterations 11 --csv "${BASELINE_CSV}" | ||
| fi | ||
| ) | ||
| git worktree remove -f "${WORKTREE_DIR}" | ||
| else | ||
| echo "Failed to create git worktree for baseline. Skipping baseline generation." | ||
| fi | ||
| else | ||
| echo "Could not find baseline commit for ${TARGET_BRANCH:-main}. Skipping baseline generation." | ||
| fi | ||
| fi | ||
|
hebaalazzeh marked this conversation as resolved.
|
||
|
|
||
| pip install -e . | ||
|
|
||
| if [ -f "${BASELINE_CSV}" ]; then | ||
| python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --fail-threshold 5000 --diff-baseline "${BASELINE_CSV}" --diff-threshold 100 | ||
| else | ||
| python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --fail-threshold 5000 | ||
| fi | ||
| retval=$? | ||
| deactivate | ||
| rm -rf .venv-profiler | ||
| rm -rf "${PROFILER_TEMP_DIR}" | ||
| else | ||
| echo "Skipping import_profile as this does not appear to be a Python package (no setup.py or pyproject.toml)." | ||
| retval=0 | ||
| fi | ||
| ;; | ||
| *) | ||
| nox -s ${TEST_TYPE} | ||
| retval=$? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR doesn't touch any packages, so it's hard to tell what the action will look like. Can you temporarily touch some files to test it out?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also see how long the new check would take if all packages were updated. In #17438, I added a new It's possible we would only want to profile a few key packages instead of all of them. Most generated libraries should be very similar
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with Daniel on a couple of performance issues:
Note In case you've never done it... clicking on the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a temporary commit (8a2d75b) that adds a blank line to Regarding the benchmark on all packages, we actually already support running the profiler against all packages in this PR! Similar to how the If we want to see how long it takes across all ~240 packages to get an order of magnitude, we can simply apply the |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4365,3 +4365,4 @@ def _get_version(dependency_name): | |
| "ZoneVmExtensionPoliciesClient", | ||
| "ZonesClient", | ||
| ) | ||
| # Trigger CI check | ||

Uh oh!
There was an error while loading. Please reload this page.