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

[Tech] System info cache #1897

Merged
merged 2 commits into from Oct 17, 2022
Merged

[Tech] System info cache #1897

merged 2 commits into from Oct 17, 2022

Conversation

BrettCleary
Copy link
Collaborator

@BrettCleary BrettCleary commented Oct 12, 2022

This improves game launch speed by caching system info that won't change while the app is running.

Time to launch was reduced from 20 seconds to 9 seconds with League of Legends on Windows.

It also speeds up Heroic launch time by ~5 seconds. Before, we were unnecessarily awaiting getSystemInfo before launching the main window. Now it runs asynchronously.


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

src/backend/main.ts Outdated Show resolved Hide resolved
@CommandMC
Copy link
Collaborator

CommandMC commented Oct 12, 2022

That 11 second metric seemed a bit high for me, so I've had a look into the systeminformation module we're using
Seems like they're executing PowerShell commands on Windows, which is why it's so slow.

Maybe we should someday implement this on our own, using Win32 API calls and/or WMI queries (there are npm modules for making both of those things easy, as far as I know). Those should be significantly faster

@BrettCleary
Copy link
Collaborator Author

BrettCleary commented Oct 12, 2022

That 11 second metric seemed a bit high for me, so I've had a look into the systeminformation module we're using Seems like they're executing PowerShell commands on Windows, which is why it's so slow.

Maybe we should someday implement this on our own, using Win32 API calls and/or WMI queries (there are npm modules for making both of those things easy, as far as I know). Those should be significantly faster

I was getting 5-10 seconds improvements consistently. Might be some user error in those numbers. Only tested on windows too

There's still no need for getSystemInfo to be synchronous and not cached. The only improvement we'd get by making the getSystemInfo call faster is if the app opens faster than getSystemInfo runs and the user launches a game before it finishes. In that case, we'd run getSystemInfo twice and the first game open would be slow. The app doesn't open that fast and I think most users won't launch a game in the first second the app opens.

It's also only used for logging so if this edge case becomes problem, we could disable system info logging on game launch since it's already logged on app start.

@CommandMC
Copy link
Collaborator

I was getting 5-10 seconds improvements consistently. Might be some user error in those numbers. Only tested on windows too

To clarify, I was not at all questioning your measurements, I was just observing & suspecting what the reason for a difference this significant might be

The app doesn't open that fast and I think most users won't launch a game in the first second the app opens.

This might happen when games are launched via the protocol

It's also only used for logging so if this edge case becomes problem, we could disable system info logging on game launch since it's already logged on app start.

This might be a bad idea, since users usually share just the game log & not the general one

@flavioislima flavioislima changed the title caching system info [Tech] System info cache Oct 17, 2022
@flavioislima flavioislima added the pr:ready-for-review Feature-complete, ready for the grind! :P label Oct 17, 2022
Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍🏽

@flavioislima flavioislima merged commit f5bdccf into beta Oct 17, 2022
@flavioislima flavioislima deleted the performance/game_launch branch October 17, 2022 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants