-
Notifications
You must be signed in to change notification settings - Fork 1.7k
perf: refactor to lazy interface loading #1932
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
perf: refactor to lazy interface loading #1932
Conversation
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.
Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:
- Convert your PR to a draft unless it's ready to review
- Read the contributing docs
- Before marking as "ready for review", ensure:
- the goal is clearly stated in the description
- all the tests are passing
- include a route or your device' dongle ID if relevant
a4b587c to
e8bfae8
Compare
| # jerk is measured over half a second | ||
| JERK_MEAS_T = 0.5 | ||
|
|
||
| @pytest.fixture |
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.
Can we do this without changing the structure of this test too much?
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.
If this is to make the review easier, yes. I reduced the diff.
It seems nonsensical, because we are not parametrizing anymore over the test class, but directly over the test functions.
70d0f30 to
72b8475
Compare
16e77b7 to
4db39c6
Compare
refactor: simplify diff
4db39c6 to
b8fe61b
Compare
|
@sshane rebased, minimized diff. |
|
This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity. |
|
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
|
This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity. |
|
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
Relates to #1184
By moving the
car_helpers.pyimports to apytest.fixture, we can shorten the collection time.Refactors the
cartests to use:fixturefor the interface loading (this import is not executed during collection)coincidentally already fixed in Replace @parameterized.expand with @pytest.mark.parametrize to improve test speed #1930pytest.mark.parametrizeinstead of theparametrizein select places to enable properly using the fixture.I tried to keep the diff as small as possible.
There are 3 big chunks of time spent during the pytest collection on a fresh run (after compilation):
CANDefine, aka the packer_pyx compiled library (only slow the first run after compilation)car_helpers.py(always slow)"Fresh" denotes a recompilation of the
cythonmodules.Collect only
pytest opendbc/car/tests --collect-onlycompare to
pytest opendbc/can/tests --collect-onlyCollection + execution
pytest opendbc/car/tests