-
Notifications
You must be signed in to change notification settings - Fork 27
Speedup import and add regression check for import time #238
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 1 commit
b6bee81
5c8032d
212d573
d4e25e7
865a4fa
82a31c4
e60c770
2777c0f
13960b4
07e084c
5c01d25
5918aeb
1161bf0
2b04381
1059be9
92a4821
23cbe32
fe1e32d
b019517
6bd0bb8
bf57cd0
acd25f9
ed48b13
2245483
f64739a
5143765
77f3f59
b0006c8
c337e54
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 |
---|---|---|
|
@@ -43,21 +43,18 @@ | |
def test_get_ref_import_time() -> None: | ||
"""A dummy test that would always fail, used to generate copyable reference time.""" | ||
import_times = { | ||
module_name: measure_import_time_in_ms(module_name) | ||
module_name: round(measure_import_time(module_name), 2) | ||
for module_name in REF_IMPORT_TIME | ||
} | ||
|
||
# Print out the import times in a copyable format | ||
print("\nCopyable import time dictionary:") | ||
print("{") | ||
for module_name, import_time in import_times.items(): | ||
print(f' "{module_name}": {import_time:.2f},') | ||
print("}") | ||
print(import_times) | ||
|
||
pytest.fail("Generate reference import times.") | ||
|
||
|
||
def measure_import_time_in_ms(module_name: str, count: int = 3) -> float: | ||
def measure_import_time(module_name: str, repeats: int = 3) -> float: | ||
"""Measure import time of a module in milliseconds across several runs. | ||
|
||
Args: | ||
|
@@ -69,12 +66,12 @@ def measure_import_time_in_ms(module_name: str, count: int = 3) -> float: | |
""" | ||
total_time = 0.0 | ||
|
||
for _ in range(count): | ||
start_time = time.perf_counter_ns() | ||
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. Thanks for catching this. using
|
||
for _ in range(repeats): | ||
start_time = time.perf_counter() | ||
subprocess.run(["python", "-c", f"import {module_name}"], check=True) # noqa: S603, S607 | ||
total_time += time.perf_counter_ns() - start_time | ||
total_time += time.perf_counter() - start_time | ||
|
||
return (total_time / count) / 1e6 | ||
return total_time / repeats * 1000 | ||
|
||
|
||
@pytest.mark.skipif( | ||
|
@@ -91,7 +88,7 @@ def test_import_time(grace_percent: float = 0.20, hard_percent: float = 0.50) -> | |
before the test fails. | ||
""" | ||
for module_name, ref_time in REF_IMPORT_TIME.items(): | ||
current_time = measure_import_time_in_ms(module_name) | ||
current_time = measure_import_time(module_name) | ||
|
||
# Calculate grace and hard thresholds | ||
grace_threshold = ref_time * (1 + grace_percent) | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part was intended to generate a directly copyable dict (and human-readable for manual inspection) for us to directly copy and paste to update the time recording, printing a dict directly gives a one-liner:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i saw that. the thing is, if you paste that printed dict into a python file,
ruff
will auto-format it to multiple lines so having easier to read code seemed more importantThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point!