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

Internal refactor TreeFS collectLinkPaths #1276

Closed
wants to merge 1 commit into from

Conversation

robhogan
Copy link
Contributor

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 May 27, 2024
@facebook-github-bot
Copy link
Contributor

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

robhogan added a commit to robhogan/metro that referenced this pull request Jun 4, 2024
…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
@robhogan robhogan force-pushed the export-D57844055 branch from 3859665 to 8ac9ec8 Compare June 4, 2024 12:48
@facebook-github-bot
Copy link
Contributor

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

robhogan added a commit to robhogan/metro that referenced this pull request Jun 4, 2024
…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
…acebook#1276)

Summary:
Pull Request resolved: facebook#1276

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

Reviewed By: huntie

Differential Revision: D57844055
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 93f26ed.

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 Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants