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 #1614

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fahhem
Copy link

@fahhem fahhem commented Jan 7, 2025

From #1599:

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".

I switched to functools.cache as requested by @sshane. Speed is still very close, might be slightly faster, but definitely in the noise. Sometimes 0.88s, sometimes 0.93s, but on master I get a similar spread. I'm not sure it's worth analyzing the time much further.

@github-actions github-actions bot added the car related to opendbc/car/ label Jan 7, 2025
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