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

Performance on Windows #113

Closed
pgreenland opened this issue Jan 22, 2024 · 7 comments
Closed

Performance on Windows #113

pgreenland opened this issue Jan 22, 2024 · 7 comments
Assignees

Comments

@pgreenland
Copy link

Hey,

Firstly thanks for an awesome library!

I've been using it on Linux and now Windows as part of an ECU firmware updater. So been using the pure Python implementation.

It's been working well on both, however performance on Windows has been terrible compared to Linux.

As an example on Linux the main file transfer takes 10s, on Windows it takes 2mins.

I believe I've narrowed it down to the Timer implementation, which calls time.monotonic_ns. The resolution of that Timer on Windows doesn't appear to be very good.

Based on the discussion of time.monotonic here https://bugs.python.org/issue44328.

Tacking the following into my code resolves the issue:

if platform.system() == "Windows":
    # On windows patch time.monotonic_ns for isotp library.
    # It seems time.monotonic uses a timer which has a resolution of 16ms
    # See: https://bugs.python.org/issue44328
    # Instead we'll override it with our own version
    time.monotonic_ns = lambda: time.perf_counter() * 1000000000.0

Bringing the update time down to around 13-15s....not quite as good as Linux....but it's close enough. Watching the CAN traffic on another machine the inter-frame delay drops from 10-12ms to 1ms as requested by the remote device.

Thought it may be beneficial to others to fix the problem at source.

Thanks,

Phil

@pylessard
Copy link
Owner

oh wow.
With isotp v2, I've spent a lot of effort to redesign the module the remove all timing weakness. This big one went unnoticed.
I will fix this very very soon.
Thank you

@pylessard
Copy link
Owner

@pylessard pylessard self-assigned this Jan 22, 2024
@pgreenland
Copy link
Author

Wow that was rapid, thanks!

I'll give it a go on both platforms later today and report back. Im wondering if those extra timers you tweaked may be responsible for my additional few seconds 😄 .

Regards,

Phil

@pylessard
Copy link
Owner

Possibly, if you use the rate limiter feature, it could be.
The other one is only a protection to avoid bloating a core, so I doubt it is.

@pgreenland
Copy link
Author

Installed on Windows and Linux (removing my hack from Windows).

Working great in both. Windows is still a tiny bit slower, but eh....its Windows you get what you pay for :-P

Thanks for sorting it so quickly.

Phil

@pylessard
Copy link
Owner

Awesome. Expect v2.0.3 very shortly

@pylessard
Copy link
Owner

Fixed in #114
Released into v2.0.3

Thank you again, that was probably the last missing piece of the timing puzzle.
Cheers

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

No branches or pull requests

2 participants