Skip to content

ci: add import profiler check across monorepo#17657

Open
hebaalazzeh wants to merge 22 commits into
mainfrom
feat/import-profiler-ci
Open

ci: add import profiler check across monorepo#17657
hebaalazzeh wants to merge 22 commits into
mainfrom
feat/import-profiler-ci

Conversation

@hebaalazzeh

@hebaalazzeh hebaalazzeh commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

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 Actions Workflow: Created a new workflow (.github/workflows/import-profiler.yml) that triggers on PRs and merge groups. The workflow is pinned specifically to Python 3.15.
  • Native CI Integration (No template bloat): Rather than polluting the central gapic-generator template (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.
  • Dynamic Differential Checks: Enhanced ci/run_single_test.sh to checkout HEAD^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.
  • Safety Backstop: The script still enforces an absolute hard-failure backstop of 5000ms for extreme regressions.
  • Conditional Execution & Graceful Skips: The pipeline verifies if a valid setup.py exists before attempting to profile, gracefully skipping directories that are not valid Python packages.
  • All Packages Testing: Adding the import_profile:all_packages label 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 --cprofile flag (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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/gapic-generator/gapic/templates/noxfile.py.j2 Outdated

@daniel-sanche daniel-sanche left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread ci/run_single_test.sh Outdated
Comment thread ci/run_single_test.sh Outdated
Comment thread ci/run_single_test.sh
;;
*)
nox -s ${TEST_TYPE}
retval=$?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed!

@chalmerlowe chalmerlowe Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Daniel on a couple of performance issues:

  1. 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:

          Screenshot 2026-07-09 at 11 38 17

  1. 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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 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.

Comment thread ci/run_single_test.sh Outdated
@hebaalazzeh hebaalazzeh force-pushed the feat/import-profiler-ci branch from 113b3d5 to 9472ab6 Compare July 8, 2026 23:50
@hebaalazzeh hebaalazzeh marked this pull request as ready for review July 9, 2026 04:09
@hebaalazzeh hebaalazzeh requested a review from a team as a code owner July 9, 2026 04:09
@hebaalazzeh

Copy link
Copy Markdown
Contributor Author

/gemini

Comment thread .github/workflows/import-profiler.yml Outdated
Comment on lines +30 to +31
python -m pip install --upgrade setuptools pip wheel
python -m pip install nox

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed

@chalmerlowe

Copy link
Copy Markdown
Contributor

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

Successfully built google-cloud-compute
Installing collected packages: google-cloud-compute
  Attempting uninstall: google-cloud-compute
    Found existing installation: google-cloud-compute 1.49.0
    Uninstalling google-cloud-compute-1.49.0:
      Successfully uninstalled google-cloud-compute-1.49.0
Successfully installed google-cloud-compute-1.49.0
usage: profiler.py [-h] [--module MODULE] [--iterations ITERATIONS]
                   [--cpu CPU] [--csv CSV] [--trace] [--cprofile] [--mprofile]
                   [--keep-pycache]
profiler.py: error: unrecognized arguments: --package google-cloud-compute
Previous HEAD position was 006c9479b chore: add reauth extra to constraints (#17685)

Later in the same stretch we see some results that indicate a failure but do so in a way that is non-intuitive:
The FAILURE line is buried in the middle of the results. Typically success and failure messages are the last OR nearly last thing to display.

CPU Pinning enabled: Pinning processes to core 0 using taskset.
--- Results for google.cloud.compute (10 iterations) ---
Code Volume (Deterministic):
  Loaded Modules: 1320
  Loaded Lines:   1023913
Time (ms):
  P50 (Median): 13919.05
  P90:          14474.85
  P99:          14981.28
  Mean:         13981.90
  Min:          13823.31
  Max:          14526.00
  StdDev:       199.01

Tracemalloc RAM (MB):
  P50 (Median): 115.4449
  P90:          115.4457
  P99:          115.4461
  Mean:         115.4448
  Min:          115.4435
  Max:          115.4457
  StdDev:       0.0006

FAILURE: P99 import time (14981.28 ms) exceeds the failure threshold (5000.0 ms).
Physical RSS RAM (MB):
  P50 (Median): 227.4062
  P90:          227.4719
  P99:          227.4796
  Mean:         227.4008
  Min:          227.3086
  Max:          227.4727
  StdDev:       0.0532

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.

Session import_profiler was successful in 2 minutes.
Session import_profiler was failed in 3 minutes.

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:

    # Distinct exit codes for CI/CD
    if <status is success>:
        sys.exit(0)
    else:
        sys.exit(1)

@hebaalazzeh

Copy link
Copy Markdown
Contributor Author

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.

  1. "The FAILURE line is buried in the middle of the results..." I've refactored the script to collect all validation checks into a final_messages array and print them sequentially at the very end of the execution. I also ensured that we are explicitly calling sys.exit(0) and sys.exit(1) so the CI step fails correctly.

  2. "profiler.py: error: unrecognized arguments: --package google-cloud-compute" This error happened because of our differential baseline checks. The CI script checks out HEAD^1 (the main branch) to measure the baseline import time. However, the profiler.py script on the main branch didn't have the --package argument yet! To fix this, the CI script now copies the new profiler.py into /tmp and runs that version while HEAD^1 is checked out, ensuring it always has the updated arguments.

  3. "I would ask that you make sure that the functionality works by applying the label and then summarize the results..." I've restored the dynamic check for the import_profile:all_packages label in .github/workflows/import-profiler.yml.

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!

@chalmerlowe

chalmerlowe commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

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 Size

Let's consider this result from one of the runs in this PR:

Time (ms):
  P50 (Median): 13919.05
  P90:          14474.85
  P99:          14981.28 *
  Mean:         13981.90
  Min:          13823.31
  Max:          14526.00 *
  StdDev:       199.01

In any calculation, the highest datapoint is the 100th percentile. In this case our max is 14526.00
The 99th percentile by definition must be lower than the 100th percentile but here it is 14981.28. This does not make sense.

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.
To be able to statisically validate the 99th percentile we typically need far more than 10 iterations. I have seen estimates of n = 300 to 500 iterations. To get really high confidence comparisons (99.9) we are looking at 1000 to 10,000 iterations.

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:

"If a single request experiences a network glitch or disk contention, its latency might jump to several seconds. This single anomaly would define the P99 latency, even if the other 296 requests were handled in milliseconds. As a result, the reported P99 latency becomes misleading."

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 mean. P90 is potentially statistical noise and P99 and Max ARE statistical noise in this case.

CPU Pinning

Looking at this: run_master(iterations, target_module, cpu=0, csv_path=None, clear_cache=True, fail_threshold=None, diff_baseline=None, diff_threshold=None)
cpu=0 means that we are pinning the process to CPU 0. This is generally a good idea because it prevents the operating system from moving the process between cores, which can cause performance variations. However, if we are running multiple benchmarks at the same time we may experience contention for that one core, which may skew results and affect our ability to compare at the P99 level.

@chalmerlowe chalmerlowe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got some questions on whether our approach and or algorithms are correct.

  • needless repetition
  • inability to rely on P99 values
  • module name handling

Comment thread ci/run_single_test.sh Outdated
fi
git checkout -
# Re-install the current branch to ensure we profile the latest code
pip install -e .

@chalmerlowe chalmerlowe Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. Create a fresh virtual environment (python3 -m venv .venv-profiler).
  2. Run pip install -e . to install the package (and its dependencies from PyPI).
  3. Check out HEAD^1 (baseline).
  4. Run pip install -e . AGAIN on the baseline code.
  5. Profile the baseline.
  6. Check out the PR code again.
  7. Run pip install -e . A THIRD TIME to restore the PR code.
  8. 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry that the find_module_from_package heuristic is too brittle. Here are some examples of where it potentially fails:

  1. proto-plus
    Package Name: proto-plus
    Generated Candidates: proto.plus, proto_plus
    The Reality: The actual module installed and used is proto.
    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.

  2. 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 a core directory. It extends google.cloud directly with modules like client, exceptions, etc.
    The Bug: The heuristic falls back to google.cloud.core. If you try to import google.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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed

@chalmerlowe

Copy link
Copy Markdown
Contributor

Benchmarking is notoriously tricky: One issue is caching at multiple levels.

1. The OS Page Cache

Python 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).
The script then runs the PR version (10 iterations).

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.
Accept that OS caching is unavoidable in CI without root privileges (to drop caches).

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)
If we truly want cold starts, we need to ensure the OS cache is dropped between runs (this might require elevated privileges since it is a shell level process... requires sync; echo 3 > /proc/sys/vm/drop_caches, which requires sudo).

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%)

@hebaalazzeh

Copy link
Copy Markdown
Contributor Author

Thanks for the follow-up, @chalmerlowe! I've pushed an additional commit to address these points:

  1. Redundant Editable Installs I've removed the extra pip install -e . calls in ci/run_single_test.sh when checking out HEAD^1. The changes are now applied immediately via the existing editable install.

  2. Caching and "Warm Starts" I've implemented Option A. The CI script now runs with --iterations 11, and profiler.py discards the first iteration as a cache burn-in run. The final metrics are calculated using only the remaining 10 warm starts.

  3. Virtual Environment Bytecode Caching I've updated clean_bytecode() to explicitly traverse into .venv-profiler while ignoring other dot-directories. The virtual environment's cache is now cleared correctly between runs.

  4. find_module_from_package Resolution I've replaced the string substitution heuristic. The function now reads package metadata via importlib.metadata.files() to resolve the underlying module, and falls back to setuptools.find_namespace_packages() when running inside editable local source trees (such as in CI). This correctly handles packages like proto-plus and google-cloud-core.

Let me know if there is anything else that needs to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants