ls: replace recursion with depth-first search#11386
Conversation
948e042 to
cca589f
Compare
Merging this PR will degrade performance by 92.89%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | ls_recursive_deep_tree[(200, 2)] |
570.5 KB | 143.2 KB | ×4 |
| ❌ | Memory | ls_recursive_balanced_tree[(6, 4, 15)] |
129.2 KB | 1,817.9 KB | -92.89% |
| ❌ | Memory | ls_recursive_long_all_balanced_tree[(6, 4, 15)] |
310.5 KB | 1,986.5 KB | -84.37% |
| ⚡ | Memory | ls_recursive_long_all_deep_tree[(100, 4)] |
448.1 KB | 126 KB | ×3.6 |
| ⚡ | Memory | ls_recursive_long_all_wide_tree[(15000, 1500)] |
14.1 MB | 9.9 MB | +42.58% |
| ❌ | Memory | ls_recursive_long_all_mixed_tree |
115.9 KB | 132.4 KB | -12.51% |
| ⚡ | Memory | ls_recursive_wide_tree[(10000, 1000)] |
8 MB | 6.6 MB | +21.09% |
| ❌ | Memory | ls_recursive_mixed_tree |
115.3 KB | 133.1 KB | -13.33% |
| ⚡ | Simulation | ls_recursive_deep_tree[(200, 2)] |
1.8 ms | 1.7 ms | +3.55% |
| ❌ | Simulation | ls_recursive_balanced_tree[(6, 4, 15)] |
52.8 ms | 57 ms | -7.32% |
| ⚡ | Simulation | ls_recursive_long_all_deep_tree[(100, 4)] |
3.1 ms | 2.9 ms | +6.19% |
| ⚡ | Simulation | ls_recursive_long_all_wide_tree[(15000, 1500)] |
143.1 ms | 136 ms | +5.21% |
Comparing Alonely0:ls-dfs-fix (c913b43) with main (3b3d5a7)2
Footnotes
-
48 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(0474e55) during the generation of this report, so 3b3d5a7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
2d14972 to
c88ade7
Compare
|
GNU testsuite comparison: |
Changes ls to use a Depth-First Search (DFS) algorithm instead of recursion. Fixes uutils#8725 and should help towards fixing uutils#11215; this also opens the door for greater optimizations that fully fix the latter.
|
GNU testsuite comparison: |
|
@sylvestre I've done some modifications to keep memory usage stable across all benchmarks, except those with combinatorial explosion (which are not very realistic, but worth taking a look at; I have already pushed an optimization for them). I'll work on perf and maybe do some refactoring in another PR, if that's good by you. I think this is a good baseline for future work. |
Fixes a regression introduced in uutils#11386 (sorry) where DFS nodes did not properly free the recorded inodes of the parent directories, causing critical errors in some instances of recursive symlinks.
Fixes a regression introduced in uutils#11386 (sorry) where DFS nodes did not properly free the recorded inodes of the parent directories, causing critical errors in some instances of recursive symlinks.
Fixes a regression introduced in uutils#11386 (sorry) where DFS nodes did not properly free the recorded inodes of the parent directories, causing critical errors in some instances of recursive symlinks.
Changes ls to use a Depth-First Search (DFS) algorithm instead of recursion. Reopened from #11384 because I closed it by mistake.
Fixes #8725 and should help towards fixing #11215; this also opens the door for greater optimizations that fully fix the latter.