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

[Enhancement] Speed up initial module imports #416

Merged
merged 5 commits into from
Aug 5, 2023

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Jul 30, 2023

Builds off of branch by @jdlcdl.

Prevent long delays on the initial Main Menu screen as well as further delays when clicking into any of the initial menu options.

  • Adds a background import thread so that lengthy (0.5 - 4s) imports can happen in the background while the normal startup sequence is displayed (logo, version num, etc).
  • Modifies how a lot of the python files do their imports to give us more control over what is imported when.
  • Removes the import * approach to seedsigner.models in its init.py.
  • Minor refactor around Controller.storage to account for access during test suite runs.
  • Minor refactor around Controller.screensaver to discourage direct access when possible.

@jdlcdl
Copy link

jdlcdl commented Jul 30, 2023

As of f30c1da:

This pr is passing all tests on non-raspi as well as pi2.

Same when integrated with on-deck PRs #380, #410, and #394
(assuming small changes noted for those PRs in those issues, also as my recent commits at the bottom of here )

I have already been thru many screens, I'll continue to do so over time... this is my temporary "dev" branch that I'll test everything on until merged.

Performance improvement is VERY impressive, it's not a trade-off, it's a noticeable net-gain.

  • tested on pi0 seedsigner-os: splash after ~14s, MainMenu after ~21s, no initial delay into a submenu.
  • tested on pi2-dev seedsigner-os: splash after ~12s, MainMenu after ~17s, no delay.

Still to do:

  • another careful code review.

Nicely done!

@newtonick
Copy link
Collaborator

Concept ACK. Did some testings and everything worked without issue. I need to review the code changes.

pytest now prints the module load time making it much less readable output.

@kdmukai
Copy link
Contributor Author

kdmukai commented Jul 31, 2023

Yes, @newtonick I'll be removing all the debugging noise as it gets closer to being ready to merge. Once I update this branch with the latest in dev, I think it should be pretty close to being ready.

@newtonick newtonick added the enhancement New feature or request label Aug 1, 2023
@newtonick newtonick added this to the 0.7.0 milestone Aug 3, 2023
@newtonick
Copy link
Collaborator

Once conflicting files are resolved I'd love to review and merge.

@kdmukai kdmukai marked this pull request as ready for review August 5, 2023 16:06
@kdmukai
Copy link
Contributor Author

kdmukai commented Aug 5, 2023

Testing this branch in a custom SeedSigner OS build seems to work as expected.

The background thread is not yet complete if you immediately select a menu option. Worst-case scenario seems to be about a 2-sec delay while it finishes loading. If you don't immediately click and let the thread complete, the next screen loads without a delay as expected.

The onscreen rendering rate definitely looks choppier while all the background processing is going on, but then the other rendering improvements in dev assert themselves when there aren't competing threads vying for resources.

@jdlcdl
Copy link

jdlcdl commented Aug 5, 2023

as of 260d282

ACK tested.

I've been watching it for some time; big changes and big improvements in this pr.
tested in raspi-os on pi2 and it's faster than ever.
tested in ss-os with unmerged pr55 on pi0. It feels like a different board lately.
Both have very little delay at startup, if any.
test suites are passing on my desktop and on the pi2, thank you for hushing all the timing (It was nice, but less interesting when they're 0.001s and below).

@newtonick newtonick merged commit 2c56308 into SeedSigner:dev Aug 5, 2023
@newtonick
Copy link
Collaborator

ACK Tested

@kdmukai kdmukai deleted the speedup_launch branch September 2, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants