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

Resolver perf: Use hierarchicalLookup for closestPackage (3/n) #1283

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robhogan
Copy link
Contributor

@robhogan robhogan commented Jun 4, 2024

Differential Revision: D58066514

robhogan added 3 commits June 4, 2024 08:38
…acebook#1276)

Summary:

This is a light refactoring of a private method to clear the way for subsequent diffs. In `TreeFS._lookupByNormalPath`, rather than returning a list of project-relative traversed symlink paths, we accept an optional `Set<string>` into which we write absolute paths.

This is more ergonomic and performant for callers that repeatedly call `_lookupByNormalPath` (the upcoming `hierarchicalLookup`) and want to aggregate the paths that could invalidate its result (for incremental resolution) - we save instantiating a new array and then copying its contents into an aggregate set for each call. 

Additionally, `_lookupByNormalPath` can perform the normal->absolute path conversion (which is cheap but not free) only if `collectLinkPaths` is given.

Changelog: Internal

Differential Revision: D57844055
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 4, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D58066514

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants