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

perf(pnp): reduce work done to find zip files and the pnp hook #1474

Merged
merged 17 commits into from
Jun 19, 2020

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Jun 13, 2020

What's the problem this PR addresses?

Running eslint with the PnP linker is a lot slower than the node_modules linker

How did you fix it?

  • Use the pnpApiPath from the parent module if it shares the same folder as the requested file
  • cache findApiPath lookup result
  • reduce fs calls in findZip

Result
Time measured using the time command

before after
yarn test:lint 1m31.802s 0m34.639s
yarn build:pnp:hook 1m49.554s 1m10.708s
vue cli build 1m27.259s 1m1.812s
vue cli build with cache 1m0.161s 0m41.528s
nextjs build 0m53.170s 0m30.616s
nextjs build with cache 0m43.098s 0m23.821s

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective.
  • I have verified that all automated PR checks pass.

@merceyz merceyz changed the title perf(pnp): use pnpApiPath from the parent if located in the same folder perf(pnp): reduce amount of work done to find zip files and the pnp hook Jun 18, 2020
@merceyz merceyz changed the title perf(pnp): reduce amount of work done to find zip files and the pnp hook perf(pnp): reduce work done to find zip files and the pnp hook Jun 18, 2020
@merceyz merceyz marked this pull request as ready for review June 18, 2020 19:47
@arcanis arcanis merged commit 71fb1aa into master Jun 19, 2020
@arcanis arcanis deleted the merceyz/optimize-hook-lookup branch June 19, 2020 13:02
@merceyz merceyz mentioned this pull request Sep 9, 2020
2 tasks
@thatsmydoing thatsmydoing mentioned this pull request May 31, 2023
3 tasks
github-merge-queue bot pushed a commit that referenced this pull request Nov 18, 2024
This is the same as #5474 but
allows edits by maintainer

**What's the problem this PR addresses?**
<!-- Describe the rationale of your PR. -->
Yarn shouldn't care if zips in the cache are symlinks or not. The only
reason it doesn't work is that finding the mount point skips zips if
they're not regular files. This used to be supported until it was
removed for "performance" reasons in
#1474. The bulk of the logic in the
linked PR involves resolving the real path of the symlink but I don't
think that's needed for this?

<!-- Link all issues that it closes. (Closes/Resolves #xxxx.) -->
Resolves #3514
Closes #5474 (supersedes it)

**How did you fix it?**
I switched the check to use `stat` from `lstat`.

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [X] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [X] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.

---------

Co-authored-by: merceyz <[email protected]>
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.

2 participants