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

Retrieve user's keymap path only once #114

Merged
merged 1 commit into from May 31, 2019

Conversation

Projects
None yet
3 participants
@as-cii
Copy link
Member

commented May 30, 2019

Previously, we would retrieve all the active key bindings and filter out the ones that were not defined by the user. The filtering code would repeatedly call atom.keymaps.getUserKeymapPath for every loaded key binding, which had the unfortunate side effect of issuing a statSync call to check whether the custom keymap file existed or not.

Screen Shot 2019-05-30 at 11 51 09

This commit changes the package to only retrieve the user's keymap path once. This consistently reduces Atom startup time by ~50ms on my SSD. Since it was almost entirely I/O work, I would imagine this having an even more significant impact on HDDs.

Screen Shot 2019-05-30 at 11 50 38

Retrieve user's keymap path only once
Previously, we would retrieve all the active key bindings and filter out 
the ones that were not defined by the user. The filtering code would 
repeatedly call `atom.keymaps.getUserKeymapPath` for every loaded key 
binding, which had the unfortunate side effect of issuing a `statSync` 
call to check whether the custom keymap file existed or not.

This commit changes the package to only retrieve the user's keymap path 
once. This consistently reduces Atom startup time by ~50ms on my SSD. 
Since it was almost entirely I/O work, I would imagine this having an 
even more significant impact on HDDs.

@as-cii as-cii requested a review from rafeca May 30, 2019

@as-cii as-cii self-assigned this May 30, 2019

@as-cii

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

Given how straightforward this is, I'll go ahead and merge it. Happy to revisit this, though, in case anybody has any concern about it.

@as-cii as-cii merged commit 2227359 into master May 31, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@as-cii as-cii deleted the as/speed-up-user-keybindings-load branch May 31, 2019

@rafeca

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

Nice catch!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.