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

Optimize performance for binder, imports and packages (Rebased from sbalabanov/master) #1711

Merged
merged 5 commits into from
Nov 13, 2021

Conversation

StevenACoffman
Copy link
Collaborator

@StevenACoffman StevenACoffman commented Nov 13, 2021

This is not my work. I just rebased #1702 against master.

This PR consists of multiple commits and is pure refactoring. It should maintain full backwards compatibility with the origin.

  • imports.goModuleRoot is called for every type resolution. It tries to find go.mod file in the directory tree making a gazillion of requests to open file. While someone more familiar with the application design can come up with a better architecture to avoid this altogether, I added a caching layer on top of this function to only traverse file system once.
  • binder.FindObject traverses all definitions in a package and for all needed types it is effectively O(n^2). Make it to O(n) by caching all package top-level definitions in a hashtable.
  • packages.Load calls 'packages.LoadAll' which allocates a slice from the heap even when a single package is required. Added a fast path by checking a cache map before calling to packages.LoadAll to avoid that. A possibly better solution is to refactor common parts of packages.Load and packages.LoadAll into a separate function.

For the targeted use case we achieved ~20% speed optimizations after those improvements.

Because functionality did not change, existing tests should cover it all so no new tests added.

@coveralls
Copy link

coveralls commented Nov 13, 2021

Coverage Status

Coverage increased (+0.2%) to 67.578% when pulling 24ed400 on sbalabanov/master into 237a7e6 on master.

Signed-off-by: Steve Coffman <steve@khanacademy.org>
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.

3 participants