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

Decrease startup-time by avoiding linear-time iteration over key mappings #851

Merged
merged 2 commits into from Jun 12, 2018

Conversation

mnussbaum
Copy link
Contributor

@mnussbaum mnussbaum commented Jun 11, 2018

This commit makes adding, removing and finding key maps an O(1) operation instead of O(n), where n is the number of pre-existing key mappings.

In my testing, averaged over 100 iterations, this reduces the time spent initializing NERDTree at Vim startup from ~73ms to ~9.7ms. That's with only the default included key maps. I've also confirmed that adding more key mappings doesn't meaningfully increase NERDTree startup time with this patch, and that it does without it.

I chose to add an internal s:KeyMap.all function to initialize and return the key map dictionary so that the public s:KeyMap.All function's signature stays the same, returning a sorted list of key maps.

Let me know if there're any revisions you'd like to see!

This commit makes adding, removing and finding key maps an O(1)
operation instead of O(n), where n is the number of pre-existing maps.

In my testing, averaged over 100 iterations, this reduces the time spent
initializing NERDTree at Vim startup from ~73ms to ~9.7ms. That's with
only the default included key maps.
@mnussbaum
Copy link
Contributor Author

I think this fixes the majority of the startup time issue #276

@mnussbaum mnussbaum changed the title Decrease startup-time by avoiding linear-time iteration over maps Decrease startup-time by avoiding linear-time iteration over key mappings Jun 11, 2018
Copy link
Member

@PhilRunninger PhilRunninger left a comment

Choose a reason for hiding this comment

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

I'm OK with this. Nice improvement.

Copy link
Contributor

@lifecrisis lifecrisis left a comment

Choose a reason for hiding this comment

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

This is really great. Well done!

I should have noticed this myself when I was poking around in here a few days ago.

Could you change all() to _all()? Private methods should begin with an underscore. It's a NERDTree style convention. Thanks!

@PhilRunninger PhilRunninger merged commit bf4591c into preservim:master Jun 12, 2018
@mnussbaum
Copy link
Contributor Author

Thanks! Just pushed 657be6b to update the private function to add the leading underscore

Copy link
Contributor

@lifecrisis lifecrisis left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants