ci: add import profiler check across monorepo#17657
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new import_profile session to the noxfile.py.j2 template to measure and ensure import times remain below defined thresholds. Feedback points out that because this template is used in standalone repositories outside of the monorepo, referencing a relative path to the monorepo's root scripts directory will cause default nox runs to fail in those environments. It is recommended to check for the script's existence and gracefully skip the session if it is missing, while also using pathlib.Path for cleaner path handling.
There was a problem hiding this comment.
I left a couple comments. It would be good to clarify the goal of this check though.
What counts as a failure? Can we calculate an import time diff before and after the target changes, instead of just reporting an absolute value? How are developers intended to interact with this?
| ;; | ||
| *) | ||
| nox -s ${TEST_TYPE} | ||
| retval=$? |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
We should also see how long the new check would take if all packages were updated. In #17438, I added a new unit_test:all_packages tag, that will run against all packages when added. Maybe we should support that here too
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
There was a problem hiding this comment.
I agree with Daniel on a couple of performance issues:
- I would like to see this thing run so that I can understand what it will produce and whether output appears as I would expect. Please temporarily touch some files to trigger the profiler to run against them. As an example: add a blank line right after the license in this file in google-cloud-compute and a similar change in at least one other package. That will trigger the profiler to run (without requiring you to go back and undo any changes to
google-cloud-compute, etc).
- If you already did this and have reverted your change, please point to the commit that you used to trigger the profiler (we should be able to check the CI/CD results for that commit).
Note
In case you've never done it... clicking on the x OR checkmark next to the commit number grants access to the test results for previous commits:
- If, as Daniel suggests, we run the profiler against all packages, it would be useful to know how long that CI/CD check will take. We have 240 generated packages and if we regen and profile all of them we don't wanna find out later that it takes hours. Do we have benchmark results to give us an order of magnitude for how long it takes?
There was a problem hiding this comment.
I've added a temporary commit (8a2d75b) that adds a blank line to google-cloud-compute and google-api-core so you can see the profiler run on a couple of real packages and review the output.
Regarding the benchmark on all packages, we actually already support running the profiler against all packages in this PR! Similar to how the unit_test:all_packages label works, there's logic in .github/workflows/import-profiler.yml that checks for an import_profile:all_packages label.
If we want to see how long it takes across all ~240 packages to get an order of magnitude, we can simply apply the import_profile:all_packages label to this PR and let it run.
113b3d5 to
9472ab6
Compare
|
/gemini |
| python -m pip install --upgrade setuptools pip wheel | ||
| python -m pip install nox |
There was a problem hiding this comment.
I might be missing it, if so, sorry. I don't see any specific references to your import profiler job using nox. We install it here, but when I check for the import_profile) case statement in run_single_test.sh` I don't see any nox references. Every other case explicitly uses nox to run their tests.
Note
Since you do not launch your script using a function from the noxfile.py, you should NOT need to install nox.
|
I reviewed the results of the profiler run. It appears as though it is partially successful and yet, halfway through each run for google-cloud-compute and google-api-core an error message appears Later in the same stretch we see some results that indicate a failure but do so in a way that is non-intuitive: If the built in profiler injects that line in its results, there may be nothing we can do about it, none-the-less, what we want to happen is for our process in its entirety to complete with either a success flag OR fail flag and have a message at the end. Something like this at the end of your profiler script could do the trick of signaling to the CI/CD workflow that the script resulted in a success OR failure result: |
|
Thanks for the detailed review @chalmerlowe! You raised some excellent points. I've pushed some fixes and ran some local benchmarks to answer your questions.
Regarding your question about how long it will take to run against all 240 packages: You were absolutely right to be concerned. Based on the profiler logs, 10 iterations of google-cloud-compute takes roughly ~2.5 to 4 minutes depending on the GitHub runner. (Note: This is measuring the unoptimized baseline. Although the PEP 0810 lazy-loading implementation was just released in gapic-generator v1.37.0 via PR #17600, google-cloud-compute has not been regenerated yet to pick up those changes. Once compute is regenerated, this runtime will drop dramatically!) Since our bash script evaluates the packages sequentially inside a single GitHub Actions worker, running across all 240 packages with these unoptimized eager imports would take between 4 to 10 hours. This means a full monorepo run would almost certainly hit the hard 6-hour GitHub Actions timeout. For now, I've left the import_profile:all_packages label functionality in place for rare/partial uses, but if we ever want to mandate a monorepo-wide baseline generation, we'll need to refactor the workflow to use a parallelized matrix strategy instead of a sequential bash loop. Let me know if you have any other questions! |
|
I have been thinking more about the math behind this work and I am not convinced we are doing this correctly. I worry about statistical noise. Statistical Noise & Sample SizeLet's consider this result from one of the runs in this PR: In any calculation, the highest datapoint is the 100th percentile. In this case our I suspect that in this case to calculate a P99, our benchmarking tool is extrapolating (guessing) data points that do not exist. It is generating a statistical hallucination. The 99th percentile in 100 datapoints is the second datapoint from the tail end of the distribution. I.e. if you sort the latencies from lowest to highest the 99th percentile would be datapoint number 99 out of 100. Which means that if the second highest datapoint is even a bit higher than expected it can have a massive effect on the calculated results. This IBM article talks about a similar issue where they were considering latency in one of their processes and concluded that with even 300 samples it was likely not enough for what they wanted to measure:
https://www.ibm.com/support/pages/why-p99-latency-metrics-are-unreliable-low-traffic-workloads RECOMMENDATION: If we want to limit ourselves to 10 iterations, we should really only be comparing P50 or the CPU PinningLooking at this: |
chalmerlowe
left a comment
There was a problem hiding this comment.
Got some questions on whether our approach and or algorithms are correct.
- needless repetition
- inability to rely on P99 values
- module name handling
| fi | ||
| git checkout - | ||
| # Re-install the current branch to ensure we profile the latest code | ||
| pip install -e . |
There was a problem hiding this comment.
If I am reading this right, it appears that we are running pip install -e . three times.
Recommendation: confirm whether three installs are necessary. If not, revise this logic to perform the minimum number of installs.
- Create a fresh virtual environment (python3 -m venv .venv-profiler).
- Run
pip install -e .to install the package (and its dependencies from PyPI). - Check out HEAD^1 (baseline).
- Run
pip install -e .AGAIN on the baseline code. - Profile the baseline.
- Check out the PR code again.
- Run
pip install -e .A THIRD TIME to restore the PR code. - Profiles the PR code.
Running pip install -e . 3 times per package is slow, especially if dependencies haven't changed.
In editable mode (-e), the package points to the directory where the raw source code sits, it is not really a fully fledged install the way the other dependency installations are.
Thus if only .py files changed between the PR and the baseline, you don't need to reinstall at all. The code changes are reflected immediately when you switch branches. You only need to reinstall if setup.py, pyproject.toml or dependencies change between the new PR and main.
There was a problem hiding this comment.
addressed
I removed the redundant PIP installs in ci/run_single_test.sh that were wasting time when swapping branches to build the baseline.
| raise argparse.ArgumentTypeError(f"'{module_name}' is not a valid Python module identifier.") | ||
| return module_name | ||
|
|
||
| def find_module_from_package(pkg): |
There was a problem hiding this comment.
I worry that the find_module_from_package heuristic is too brittle. Here are some examples of where it potentially fails:
-
proto-plus
Package Name: proto-plus
Generated Candidates:proto.plus,proto_plus
The Reality: The actual module installed and used isproto.
The Bug: The heuristic will never guess proto. It will fail to find proto.plus or proto_plus via find_spec (even after installation) and will fall back to proto.plus. The profiler will then attempt to import proto.plus, which will FAIL because there is no plus submodule inside proto. -
google-cloud-core
Package Name: google-cloud-core
Generated Candidates:google.cloud.core
The Reality: Looking at the source, this package doesn't place its code in acoredirectory. It extendsgoogle.clouddirectly with modules likeclient,exceptions, etc.
The Bug: The heuristic falls back togoogle.cloud.core. If you try to importgoogle.cloud.core, it may fail because that module path doesn't exist as a distinct entity in the way the heuristic expects.
Recommendation:
Instead of guessing the module name with a heuristic, consider:
Reading the module name directly from the package metadata (e.g., using importlib.metadata or parsing pyproject.toml/setup.py).
|
Benchmarking is notoriously tricky: One issue is caching at multiple levels. 1. The OS Page CachePython profiling tools (time.perf_counter(), importlib, etc.) measure time, but they cannot control or reset the Operating System's file cache (Page Cache). What happens in the script:The script runs the Baseline (10 iterations). It imports the package and its dependencies. The OS reads these files from disk and keeps them in RAM (cache). When the PR version runs, many of the files (especially common dependencies like protobuf, grpc, or google-api-core) are already sitting in RAM from the Baseline run Result:the PR run might appear artificially faster than the Baseline simply because it got to read from RAM (Warm) while the Baseline had to read from disk (Cold). This can hide performance regressions! 2. The Python Bytecode Cache (.pyc)What happens in the script:The author tried to account for this in profiler.py with clean_bytecode(), but there might be a bug in their logic! The Code: # profiler.py
for root, dirs, files in os.walk('.'):
dirs[:] = [d for d in dirs if not d.startswith('.')] # This line skips hidden dirs!In ci/run_single_test.sh, the virtual environment is created as .venv-profiler (a hidden directory). Because clean_bytecode() skips directories starting with a dot, it will NOT delete the pycache files inside the virtualenv. Result: Dependencies installed in the venv will keep their pre-compiled bytecode across runs, meaning you aren't truly measuring a "Cold" Python startup.Recommendations for consideration:If we want reliable results, we have two main options: Option A: Focus ONLY on "Warm" Starts. How: Run the profiler 11 times, but discard the first run as a "burn-in" run to warm up the cache. Measure only runs 2-11. This ensures both Baseline and PR are compared on equal "warm" footing. Option B: True "Cold" Starts (Harder in CI) Cold starts might be necessary if we want to simulate users loading our packages in a serverless environment or Google Cloud Functions. But then again, warm may be satisfactory if we are predominantly worried less on total time to start versus a change to the time to start. (i.e. placing less emphasis on < 5000 ms and more on the diff between PR and main is < x%) |
… module discovery
|
Thanks for the follow-up, @chalmerlowe! I've pushed an additional commit to address these points:
Let me know if there is anything else that needs to be addressed. |

Overview
This PR integrates the import profiler script (introduced in #17467) into our automated CI pipeline.
Why this matters: Import times significantly impact CLI responsiveness and cold starts for Serverless products like Cloud Run and Cloud Functions. Ideally, library imports should stay under 500ms, and anything taking over 1 second is a target for optimization. This CI check helps us proactively track metrics like import latency, memory footprint, and code volume to prevent performance regressions on critical libraries.
The primary goal of this check is to track and enforce performance standards for package import times across the repository, especially following our recent work on lazy loading and cold-start optimizations.
By running this benchmark as a CI check with a defined dynamic differential failure threshold, we can programmatically prevent latency regressions in module initialization times before they are merged, ensuring downstream consumers aren't impacted by unexpectedly slow startup times.
Changes Included
.github/workflows/import-profiler.yml) that triggers on PRs and merge groups. The workflow is pinned specifically to Python 3.15.noxfile.py.j2) and forcing updates across 150+ packages, the CI script (ci/run_single_test.sh) natively handles the profiler. It automatically spins up a lightweight virtual environment, installs the target package, and runs the profiler.ci/run_single_test.shto checkoutHEAD^1(the main branch), generate a baseline CSV profile, and then diff it against the PR branch. If the P99 import time of the PR degrades by >100ms compared to the baseline, the CI check will fail.setup.pyexists before attempting to profile, gracefully skipping directories that are not valid Python packages.import_profile:all_packageslabel to a Pull Request will force the CI pipeline to run the profiler check against all packages in the mono-repo, instead of just the modified ones.Developer Interactions
If a developer fails this CI check due to an import latency regression, they can reproduce and debug it locally by running the profiler script with the
--cprofileflag (python scripts/import_profiler/profiler.py --package <their-package> --cprofile). This will generate a cProfile stack trace breakdown of the import time, allowing them to pinpoint exactly which new dependency or module initialization is causing the latency spike.Related PRs
Builds upon the import profiler tool added in #17467