Skip to content
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

Improve get_interface_attr speed through caching #1599

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fahhem
Copy link

@fahhem fahhem commented Dec 23, 2024

Inspired by #1184 but only a small step.

To me, it looks like the car/ module does a lot of (potentially unnecessary) work at import time, such as defining dataclasses for documentation of a car. As a result, collecting the tests has to not only import class and function definitions, but also do real work.

In this case, interfaces.get_interface_attr() walks a folder structure ignoring some folders, dynamically imports modules, does some optional dict-merging, and returns. During test collection, it's called 3 times, each time walking the directory structure and importing the same module.

It seems to me that actually closing #1184 would entail a dozen or more of PRs like this: finding a small improvement to the import-time work or moving it either to runtime or maybe even "compilation".

@github-actions github-actions bot added the car related to opendbc/car/ label Dec 23, 2024
Copy link
Contributor

@github-actions github-actions bot left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
car related to opendbc/car/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant