Skip to content

Commit 56b6bd2

Browse files
hebaalazzehparthea
andauthored
fix(ci): skip loaded lines calculation on iterations after the first (#17790)
This PR optimizes the pure execution latency and reduces redundant disk I/O of the import profiler script by calculating the deterministic codebase statistics (loaded lines count) only on the first iteration. ### Problem For each package profiled, the script executes 11 iterations for the baseline commit and 11 iterations for the PR commit (22 iterations total). On every single iteration, the worker process scans and counts the lines of every imported `.py` file (across standard library and third-party dependencies like `grpc`, `protobuf`, `google-api-core`, etc.). This causes redundant and expensive disk I/O. ### Solution - **First Iteration Only**: The first worker run profiles and counts the loaded line count as a baseline. - **Skip Subsequent Counting**: For iterations 2-11 (`i > 0`), the master passes `--skip-line-count` to the worker process. The worker skips the file-reading loop entirely and returns `-1` for `loaded_lines`. - **Improved Logging**: Worker process `stderr` is now forwarded to the parent master process so that warnings are visible in GHA logs. ### Benchmarks (Timing the line-counting loop itself) - **Large Packages (`google-cloud-compute`)**: - Loaded Modules: 1,414 modules (1,056,448 lines of code) - Loop overhead: **`~310 ms`** per run. - **Savings**: **`~6.2 seconds`** of pure file-open and line-reading operations saved across 22 runs. - **Small Packages (`google-auth`)**: - Loaded Modules: 17 modules (9,186 lines of code) - Loop overhead: **`~2.8 ms`** per run. Fixes #17792 --------- Co-authored-by: Anthonios Partheniou <partheniou@google.com>
1 parent 08f21a6 commit 56b6bd2

3 files changed

Lines changed: 835 additions & 79 deletions

File tree

.github/workflows/import-profiler.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ jobs:
2828
python-version: "3.15"
2929
allow-prereleases: true
3030
cache: 'pip'
31+
- name: Run import profiler unit tests
32+
run: |
33+
python -m pip install --upgrade pip
34+
pip install pytest pytest-cov setuptools
35+
pytest scripts/import_profiler/test_profiler.py --cov=profiler --cov-report=term-missing --cov-fail-under=100
3136
- name: Run import profiler
3237
env:
3338
BUILD_TYPE: presubmit

scripts/import_profiler/profiler.py

Lines changed: 89 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def get_rss_mb():
4040
except Exception:
4141
return 0.0
4242

43-
def run_worker(target_module):
43+
def run_worker(target_module, skip_line_count=False):
4444
"""Performs ONE import and returns metrics."""
4545
tracemalloc.start()
4646
importlib.invalidate_caches()
@@ -66,33 +66,36 @@ def run_worker(target_module):
6666
new_modules = modules_after - modules_before
6767

6868
loaded_lines = 0
69-
for m in new_modules:
70-
try:
71-
file_path = sys.modules[m].__file__
72-
if not file_path:
73-
continue
69+
if not skip_line_count:
70+
for m in new_modules:
71+
try:
72+
file_path = sys.modules[m].__file__
73+
if not file_path:
74+
continue
7475

75-
if file_path.endswith('.pyc'):
76-
try:
77-
file_path = importlib.util.source_from_cache(file_path)
78-
except ValueError:
79-
# Raised if the .pyc path does not follow standard PEP 3147/488 conventions.
80-
# We pass silently because the unresolved file_path will still end in '.pyc',
81-
# meaning the subsequent '.endswith('.py')' check will fail and safely skip
82-
# trying to count lines in a binary file.
83-
pass
84-
if file_path.endswith('.py'):
85-
try:
86-
with open(file_path, 'r', encoding='utf-8', errors='ignore') as f:
87-
loaded_lines += sum(1 for _ in f)
88-
except OSError as e:
89-
print(f"WARNING: Failed to read lines from {file_path}: {e}", file=sys.stderr)
90-
except KeyError:
91-
# Module disappeared from sys.modules during execution
92-
pass
93-
except AttributeError:
94-
# Module has no __file__ attribute (likely a C-extension or built-in)
95-
pass
76+
if file_path.endswith('.pyc'):
77+
try:
78+
file_path = importlib.util.source_from_cache(file_path)
79+
except ValueError:
80+
# Raised if the .pyc path does not follow standard PEP 3147/488 conventions.
81+
# We pass silently because the unresolved file_path will still end in '.pyc',
82+
# meaning the subsequent '.endswith('.py')' check will fail and safely skip
83+
# trying to count lines in a binary file.
84+
pass
85+
if file_path.endswith('.py'):
86+
try:
87+
with open(file_path, 'r', encoding='utf-8', errors='ignore') as f:
88+
loaded_lines += sum(1 for _ in f)
89+
except OSError as e:
90+
print(f"WARNING: Failed to read lines from {file_path}: {e}", file=sys.stderr)
91+
except KeyError:
92+
# Module disappeared from sys.modules during execution
93+
pass
94+
except AttributeError:
95+
# Module has no __file__ attribute (likely a C-extension or built-in)
96+
pass
97+
else:
98+
loaded_lines = -1
9699

97100
# Output to stdout for the Master to capture
98101
metrics = {
@@ -106,6 +109,8 @@ def run_worker(target_module):
106109

107110
def _run_worker_and_parse(cmd):
108111
result = subprocess.run(cmd, capture_output=True, text=True, check=True)
112+
if result.stderr.strip():
113+
print(result.stderr.strip(), file=sys.stderr)
109114
try:
110115
lines = result.stdout.strip().splitlines()
111116
data = None
@@ -186,6 +191,8 @@ def run_master(iterations, target_module, cpu=0, csv_path=None, clear_cache=True
186191
cmd += ["taskset", "-c", str(cpu)]
187192

188193
cmd += python_exe + [__file__, "--worker", f"--module={target_module}"]
194+
if i > 0:
195+
cmd += ["--skip-line-count"]
189196

190197
try:
191198
data = _run_worker_and_parse(cmd)
@@ -195,11 +202,12 @@ def run_master(iterations, target_module, cpu=0, csv_path=None, clear_cache=True
195202
print(f"Iteration {i+1}/{iterations} completed in {data['time_ms']:.2f} ms")
196203
if i > 0 and loaded_modules_val != data["loaded_modules"]:
197204
print(f"WARNING: Non-deterministic import behavior! Iteration {i+1} loaded {data['loaded_modules']} modules (expected {loaded_modules_val}).", file=sys.stderr)
198-
if i > 0 and loaded_lines_val != data["loaded_lines"]:
199-
print(f"WARNING: Non-deterministic import behavior! Iteration {i+1} loaded {data['loaded_lines']} lines (expected {loaded_lines_val}).", file=sys.stderr)
200205

201206
loaded_modules_val = data["loaded_modules"]
202-
loaded_lines_val = data["loaded_lines"]
207+
if data["loaded_lines"] == -1:
208+
data["loaded_lines"] = loaded_lines_val
209+
else:
210+
loaded_lines_val = data["loaded_lines"]
203211
except FileNotFoundError as e:
204212
if cpu != NO_CPU_PINNING and cmd and cmd[0] == "taskset":
205213
print("ERROR: 'taskset' command not found. CPU pinning is enabled but taskset is not installed. "
@@ -362,63 +370,64 @@ def run_mprofile(target_module):
362370
if p.exitcode != 0:
363371
print(f"Error generating memory snapshot, process exited with code {p.exitcode}", file=sys.stderr)
364372

365-
if __name__ == "__main__":
373+
def validate_module_name(module_name):
374+
"""Validates that the input is a structurally valid Python module identifier to prevent arbitrary code execution."""
366375
import argparse
376+
if not all(part.isidentifier() for part in module_name.split('.')):
377+
raise argparse.ArgumentTypeError(f"'{module_name}' is not a valid Python module identifier.")
378+
return module_name
367379

368-
def validate_module_name(module_name):
369-
"""Validates that the input is a structurally valid Python module identifier to prevent arbitrary code execution."""
370-
if not all(part.isidentifier() for part in module_name.split('.')):
371-
raise argparse.ArgumentTypeError(f"'{module_name}' is not a valid Python module identifier.")
372-
return module_name
380+
def find_module_from_package(pkg):
381+
import importlib.metadata
382+
import importlib.util
373383

374-
def find_module_from_package(pkg):
375-
import importlib.metadata
376-
import importlib.util
384+
# 1. Try to use importlib.metadata.files (works for standard installations from PyPI/wheels)
385+
try:
386+
files = importlib.metadata.files(pkg)
387+
if files:
388+
init_files = [str(f) for f in files if str(f).endswith('__init__.py') and '__pycache__' not in str(f) and not str(f).startswith('tests/')]
389+
if init_files:
390+
from pathlib import Path
391+
shortest_init = min(init_files, key=lambda p: len(Path(p).parts))
392+
parts = Path(shortest_init).parent.parts
393+
mod = '.'.join(parts)
394+
if importlib.util.find_spec(mod):
395+
return mod
396+
except Exception:
397+
pass
377398

378-
# 1. Try to use importlib.metadata.files (works for standard installations from PyPI/wheels)
379-
try:
380-
files = importlib.metadata.files(pkg)
381-
if files:
382-
init_files = [str(f) for f in files if str(f).endswith('__init__.py') and '__pycache__' not in str(f) and not str(f).startswith('tests/')]
383-
if init_files:
384-
from pathlib import Path
385-
shortest_init = min(init_files, key=lambda p: len(Path(p).parts))
386-
parts = Path(shortest_init).parent.parts
387-
mod = '.'.join(parts)
388-
if importlib.util.find_spec(mod):
389-
return mod
390-
except Exception:
391-
pass
399+
# 2. Try setuptools.find_namespace_packages() in current directory (works for editable installs in source trees)
400+
try:
401+
import setuptools
402+
import os
403+
if os.path.exists('setup.py') or os.path.exists('pyproject.toml'):
404+
pkgs = setuptools.find_namespace_packages(where='.')
405+
for p in sorted(pkgs, key=len):
406+
if p in ("google", "google.cloud") or p.startswith("tests"):
407+
continue
408+
path = p.replace('.', os.sep)
409+
if os.path.isfile(os.path.join(path, '__init__.py')):
410+
if importlib.util.find_spec(p):
411+
return p
412+
except Exception:
413+
pass
392414

393-
# 2. Try setuptools.find_namespace_packages() in current directory (works for editable installs in source trees)
415+
# 3. Fallback to basic string manipulation heuristics
416+
candidates = [
417+
pkg.replace('-', '.'),
418+
'.'.join(pkg.split('-')[:-1]) + '_' + pkg.split('-')[-1] if '-' in pkg else pkg,
419+
pkg.replace('-', '_')
420+
]
421+
for mod in candidates:
394422
try:
395-
import setuptools
396-
import os
397-
if os.path.exists('setup.py') or os.path.exists('pyproject.toml'):
398-
pkgs = setuptools.find_namespace_packages(where='.')
399-
for p in sorted(pkgs, key=len):
400-
if p in ("google", "google.cloud") or p.startswith("tests"):
401-
continue
402-
path = p.replace('.', os.sep)
403-
if os.path.isfile(os.path.join(path, '__init__.py')):
404-
if importlib.util.find_spec(p):
405-
return p
423+
if importlib.util.find_spec(mod):
424+
return mod
406425
except Exception:
407426
pass
427+
return candidates[0]
408428

409-
# 3. Fallback to basic string manipulation heuristics
410-
candidates = [
411-
pkg.replace('-', '.'),
412-
'.'.join(pkg.split('-')[:-1]) + '_' + pkg.split('-')[-1] if '-' in pkg else pkg,
413-
pkg.replace('-', '_')
414-
]
415-
for mod in candidates:
416-
try:
417-
if importlib.util.find_spec(mod):
418-
return mod
419-
except Exception:
420-
pass
421-
return candidates[0]
429+
if __name__ == "__main__":
430+
import argparse
422431

423432
parser = argparse.ArgumentParser(description="Python SDK Import Profiler")
424433
group = parser.add_mutually_exclusive_group(required=True)
@@ -436,6 +445,7 @@ def find_module_from_package(pkg):
436445
parser.add_argument("--diff-baseline", help="Path to a baseline CSV file to compare against.")
437446
parser.add_argument("--diff-threshold", type=float, default=100.0, help="Fail if Median time exceeds baseline Median by this many ms.")
438447
parser.add_argument("--worker", action="store_true", help=argparse.SUPPRESS)
448+
parser.add_argument("--skip-line-count", action="store_true", help=argparse.SUPPRESS)
439449

440450
args = parser.parse_args()
441451

@@ -444,7 +454,7 @@ def find_module_from_package(pkg):
444454
target_module = find_module_from_package(args.package)
445455

446456
if args.worker:
447-
run_worker(target_module)
457+
run_worker(target_module, skip_line_count=args.skip_line_count)
448458
elif args.trace:
449459
if not args.keep_pycache: clean_bytecode()
450460
run_trace(target_module)

0 commit comments

Comments
 (0)