perf(pkgs-graph): speed up createPkgGraph when directory specifiers are present #6317
Conversation
| function getPkgMapByDir (pkgMapValues: Package[]) { | ||
| const pkgMapByDir: Record<string, Package | undefined> = {} | ||
| for (const pkg of pkgMapValues) { | ||
| pkgMapByDir[path.resolve(pkg.dir)] = pkg |
There was a problem hiding this comment.
This resolve is defensive; since we're comparing resolved paths above, I wanted to ensure everything went through this algorithm before comparison. It's possible that this is not needed, however, there doesn't seem to be any negative performance hit.
|
Clearly I celebrated too early; Windows seems to be misbehaving. I'll take a look. |
|
Hm, seems like a spurious failure; I just reran it and it passed. |
|
Unfortunately there are one or two unstable tests. Especially on Windows. |
| return found.dir | ||
| } | ||
|
|
||
| // Slow path; only needed when there are case mismatches on case-insensitive filesystems. |
There was a problem hiding this comment.
In theory we could detect whether it is a case-insensitive fs. In a future optimization. I already use this lib for detecting case sensitive dirs: https://github.com/zkochan/packages/tree/main/dir-is-case-sensitive
There was a problem hiding this comment.
Yeah, that's a good idea. Another related optimization would be to ensure that all paths are "resolved", that way all of the path checks can skip re-resolving and instead use plain comparison and path.join + path.normalize.
…re present (#6317) * perf(pkgs-graph): speed up createPkgGraph when directory specifiers are present * perf(pkgs-graph): make pkgMapByManifestName equally lazy
This speeds up the install of "with paths" test case (https://github.com/jakebailey/DefinitelyTyped/tree/blog-pnpm-workspaces-with-paths) from 49.7s to 14s.
Time spent in
createPkgMaphas been reduced from ~35s to ~200ms, matching the performance in #6287.Fixes #6281 (for real this time)