Improve import startup with lazy top-level exports#2950
Improve import startup with lazy top-level exports#2950fede-kamel wants to merge 2 commits intoopenai:mainfrom
Conversation
|
Validation summary for #2819:
Interpretation: default lazy mode reduces import startup work significantly; eager mode remains available for CI/dev verification. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bd0b97246
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Two things, about the env var for eager load
I think it's great you're working on a fix for this btw... thank you |
|
Took this feedback and removed the runtime eager-import env var path from The branch now keeps lazy imports as the only product behavior and makes the coverage explicit in tests instead:
Validated locally with:
|
|
@RobertCraigie @rachellim @g0t4 — Friendly ping! Just re-validated the branch: all tests pass ( |
fede-kamel
force-pushed
the
feat/import-startup-lazy-2819
branch
from
2cb445c to
b0fb0f4
Compare
|
Friendly bump for maintainer review — rebased onto What changed since last review
Integration benchmark (Python 3.13, cold
|
| metric | origin/main (2.32.0) |
this branch | speedup |
|---|---|---|---|
| min | 346 ms | 288 ms | 1.20× |
| median | 401 ms | 325 ms | 1.23× |
| mean | 397 ms | 338 ms | 1.18× |
Python's own -X importtime cumulative time for the openai module:
origin/main: 390 ms- this branch: 232 ms → 1.68× faster / ~158 ms shaved
Repro: python scripts/bench_import.py (also in the branch).
What still stays lazy after the rebase
Verified on import openai:
openai.lib.azure— not loadedopenai.lib.streaming— not loadedopenai.lib._tools— not loaded
These are the biggest non-transitively-required submodules, which is where the ~160 ms comes from.
Happy to split further or squash to the original 3 commits if preferred. Would love to get this in — the import-time win is meaningful for CLI / Lambda / short-lived workers.
|
@mcgrew-oai @hintz-openai — friendly ping in case this slipped off the review queue. Just rebased onto 2.32.0 (see benchmark comment above: ~1.18–1.23× faster cold |
|
Friendly bump for Latest benchmark on the rebased branch (Python 3.13, cold
No API surface change — Happy to split commits, narrow scope, or close and reopen with smaller diff if it'd help review velocity. |
fede-kamel
force-pushed
the
feat/import-startup-lazy-2819
branch
from
b0fb0f4 to
66532e1
Compare
|
@fede-kamel you'd think maybe @codex could review/fix/merge this one... after all, we're getting replaced any day now per Dario |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66532e1c09
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| from ._parsing import ResponseFormatT as ResponseFormatT | ||
|
|
||
|
|
||
| def __getattr__(name: str) -> Any: |
There was a problem hiding this comment.
The lazy conversion drops pydantic_function_tool and ResponseFormatT from from openai.lib import * because these names now exist only behind __getattr__, and wildcard imports do not call __getattr__ unless __all__ is defined. Before this change both symbols were bound at module import time, so code relying on wildcard imports will now silently miss them.
Useful? React with 👍 / 👎.
| globals()[name] = value | ||
| return value | ||
|
|
||
| raise AttributeError(f"module {__name__!r} has no attribute {name!r}") |
There was a problem hiding this comment.
This AttributeError now interpolates __name__, but earlier in the module a loop assigns for __name in __all__, so __name__ is no longer 'openai' by the time this runs. As a result, unknown-attribute failures can report the wrong module name, which makes debugging and import error messages misleading.
Useful? React with 👍 / 👎.
fede-kamel
force-pushed
the
feat/import-startup-lazy-2819
branch
from
66532e1 to
0c6ff1a
Compare
|
@apcha-oai @hintz-openai — bumping for a look when you have a moment. Status: Rebased cleanly onto
Benchmarks on the rebased branch (Linux/WSL, Python 3.12, 10 cold imports each):
Modules deferred at import time on the lazy path vs. baseline:
Tests: Genuine question: is there a reason not to take a free import-time speedup that's behind an opt-in flag for safety? Happy to adjust the surface or split the change if there's a smaller slice that would be easier to land. |
2 participants
Summary
This PR addresses import-time startup regression tracked in #2819 by making heavy top-level exports lazy.
Key changes:
openai.__init__for:AzureOpenAIAsyncAzureOpenAIpydantic_function_toolAssistantEventHandlerAsyncAssistantEventHandlerOPENAI_EAGER_IMPORT=1scripts/bench_import.py.tests/test_import_surface.pytests/lib/test_import_surface_live.py(live-gated)Why this relates to #2819
Issue #2819 reports that
import openaiis too slow and that eager loading of type-heavy/internal surfaces is a major contributor. This PR reduces startup cost in the default path by moving expensive imports behind first-use access while preserving API compatibility.Testing done
Unit / behavior tests
PYTHONPATH=src python3 -m pytest tests/test_import_surface.py -q -o addopts=''-> passedPYTHONPATH=src python3 -m pytest tests/test_module_client.py -q -o addopts=''-> passedLive API-key integration test
OPENAI_LIVE=1 PYTHONPATH=src OPENAI_API_KEY=... python3 -m pytest tests/lib/test_import_surface_live.py -q -o addopts=''-> passedclient.models.list()).Performance benchmarks
Using
scripts/bench_import.py:0.1533sOPENAI_EAGER_IMPORT=1):0.2601s+0.1068s(+69.7%)0.7019s2.9429s+2.2410s(+319.3%)Interpretation: eager mode is intentionally slower, and the default lazy path materially reduces startup work compared with forced eager loading.
Notes
OPENAI_LIVE=1andOPENAI_API_KEY.