perf(nm): improve hoist traverse logic and memory usage

[WooWan]

What's the problem this PR addresses?

This PR improves the performance of getSortedRegularDependencies by reusing a single seenDeps Set across recursive calls instead of creating new ones, and using Map entries for direct key access on large dependency trees with many nested packages.

How did you fix it?

The previous code traversed nodes redundantly by recreating seenDeps for every dependency, causing repeated processing of already visited nodes. By declaring seenDeps at the function level, we eliminate unnecessary traversals and reduce memory allocations while maintaining the same results as before.

refers: #1717 (comment)

node_modules hoisting algorithm recipe
The children of root node are already "hoisted", so you need to start from the dependencies of these children. You take some child and sort its dependencies so that regular dependencies without peer dependencies will come first and then those dependencies that peer depend on them. This is needed to make algorithm more efficient and hoist nodes which are easier to hoist first and then handle peer dependent nodes.

Perfomance improvement

This is just a local benchmark for reference

When there is cache, 7% faster.

hyperfine \
  --warmup 1 \
  --prepare 'cd yarn-stable \
  --prepare 'cd yarn-after \
  'cd yarn-stable && YARN_NODE_LINKER=node-modules yarn add gatsby --mode=skip-build' \
  'cd yarn-after && YARN_NODE_LINKER=node-modules yarn add gatsby --mode=skip-build'
  
Benchmark 1: cd yarn-stable && YARN_NODE_LINKER=node-modules yarn add gatsby --mode=skip-build
  Time (mean ± σ):      1.462 s ±  0.063 s    [User: 1.673 s, System: 0.228 s]
  Range (min … max):    1.409 s …  1.620 s    10 runs
 
Benchmark 2: cd yarn-after && YARN_NODE_LINKER=node-modules yarn add gatsby --mode=skip-build
  Time (mean ± σ):      1.564 s ±  0.041 s    [User: 1.787 s, System: 0.245 s]
  Range (min … max):    1.485 s …  1.622 s    10 runs
 
Summary
  cd yarn-stable && YARN_NODE_LINKER=node-modules yarn add gatsby --mode=skip-build ran
    1.07 ± 0.05 times faster than cd yarn-after && YARN_NODE_LINKER=node-modules yarn add gatsby --mode=skip-build

Memory usage

I used time for benchmark.

Metric	yarn-stable	yarn-after	Difference
Maximum Memory (RSS)	352.9 MB	351.1 MB	-1.8 MB (-0.5%)
Maximum Memory (Peak)	256.9 MB	40.4 MB	-216.5 MB (-84.3%)

System Resource Metrics

Metric	yarn-stable	yarn-after	Difference
Page reclaims	25,791	32,218	+6,427 (+24.9%)
Page faults	2,333	13	-2,320 (-99.4%)
Voluntary context switches	1,679	1,718	+39 (+2.3%)
Involuntary context switches	27,445	25,621	-1,824 (-6.6%)
Instructions retired	15,729,559,162	847,301,569	-14,882,257,593 (-94.6%)
Cycles elapsed	6,424,724,640	324,691,231	-6,100,033,409 (-95.0%)

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[WooWan]

workflow failure has nothing to do with this change.(related: #6776)
I think this PR is ready for review!

[larixer]

The benchmarks looks strange. So yarn-after is 7% SLOWER than yarn-stable according to your output. But yarn-after has lower memory footprint, according to which tool?

[larixer]

I don't see any significant improvements with this PR on my local machine, neither in performance, nor in memory usage. The usual drift in performance and memory usage seems to be higher than the impact of the change. I'm reluctant to merge this PR, unless I see the real proof that it improves anything with proper benchmarks that can be reproduced.

[larixer]

You are changing hoisting algorithm which runs a small fraction of install time, but you are measuring the whole install time.
cd yarn-stable && YARN_NODE_LINKER=node-modules yarn add gatsby --mode=skip-build ran
If install time is 2 seconds, but hoisting time is 150ms, it is understood that such a measurement is not very good and will have huge drift.

To see how much is the hoisting time fraction you can use NM_DEBUG_LEVEL=1 yarn ....

[WooWan]

oops.. it looks strange to me too and I can see my hyperfine cli is incorrect. I will look into it, thanks

[WooWan]

Thank you for your kind explanation. You can ignore my previous benchmarks as they were inaccurate. My bad for the confusion. I measured the benchmarks again, and there was no measurable significant improvement.

Benchmark for hoist time

yarn-stable

Total runs: 10
Average hoist time: 51ms
Minimum hoist time: 50ms
Maximum hoist time: 53ms

yarn-after

Total runs: 10
Average hoist time: 51ms
Minimum hoist time: 49ms
Maximum hoist time: 53ms

[larixer]

@WooWan Thank you! This is what I see as well.

[WooWan]

I'm reluctant to merge this PR, unless I see the real proof that it improves anything with proper benchmarks that can be reproduced.

I understand, since there's no clear performance improvement. I'll close the PR for now. Thanks