Skip to content

ls: replace recursion with depth-first search #11384

Closed
Alonely0 wants to merge 0 commit intouutils:mainfrom
Alonely0:ls-dfs
Closed

ls: replace recursion with depth-first search #11384
Alonely0 wants to merge 0 commit intouutils:mainfrom
Alonely0:ls-dfs

Conversation

@Alonely0
Copy link
Copy Markdown
Contributor

Changes ls to use a Depth-First Search (DFS) algorithm instead of recursion. Fixes #8725 and sh.

Fixes #8725 and should help towards fixing #11215; this also opens the door for greater optimizations that fully fix the latter.

@Alonely0
Copy link
Copy Markdown
Contributor Author

Alonely0 commented Mar 18, 2026

CC: @oech3 @kimono-koans. I did try to use walkdir but finally could not get its ordering to be consistent with GNU and overall play nicely with our sorting code. It also made the codebase significantly more gnarly (even more so lol).

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 18, 2026

Merging this PR will degrade performance by 93.88%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 9 improved benchmarks
❌ 6 regressed benchmarks
✅ 283 untouched benchmarks
⏩ 48 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation ls_recursive_deep_tree[(200, 2)] 1.8 ms 1.5 ms +23.3%
Simulation ls_recursive_balanced_tree[(6, 4, 15)] 52.8 ms 50.8 ms +3.94%
Simulation ls_recursive_long_all_mixed_tree 2.7 ms 2.6 ms +5.49%
Simulation ls_recursive_long_all_balanced_tree[(6, 4, 15)] 136.5 ms 129.4 ms +5.45%
Simulation ls_recursive_long_all_deep_tree[(100, 4)] 3.1 ms 2.7 ms +14.25%
Simulation ls_recursive_mixed_tree 1.4 ms 1.3 ms +3.24%
Simulation ls_recursive_long_all_wide_tree[(15000, 1500)] 143.1 ms 132.9 ms +7.67%
Memory ls_recursive_long_all_wide_tree[(15000, 1500)] 14.1 MB 41.4 MB -65.88%
Memory ls_recursive_mixed_tree 115.3 KB 708.2 KB -83.71%
Memory ls_recursive_wide_tree[(10000, 1000)] 8 MB 36.9 MB -78.26%
Memory ls_recursive_deep_tree[(200, 2)] 570.5 KB 160 KB ×3.6
Memory ls_recursive_long_all_balanced_tree[(6, 4, 15)] 310.5 KB 2,279.6 KB -86.38%
Memory ls_recursive_balanced_tree[(6, 4, 15)] 129.2 KB 2,111.5 KB -93.88%
Memory ls_recursive_long_all_deep_tree[(100, 4)] 448.1 KB 134 KB ×3.3
Memory ls_recursive_long_all_mixed_tree 115.9 KB 707 KB -83.61%

Comparing Alonely0:ls-dfs (c31cb0d) with main (5dcde30)

Open in CodSpeed

Footnotes

  1. 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.

@sylvestre
Copy link
Copy Markdown
Contributor

the typical - you trade memory for performance :)

@Alonely0
Copy link
Copy Markdown
Contributor Author

the typical - you trade memory for performance :)

yeah it's my fault for forgetting to drop file descriptors early hahaha. I'm working on a fix.

@Alonely0
Copy link
Copy Markdown
Contributor Author

Alonely0 commented Mar 18, 2026

wtf I must've closed it by mistake. It won't let me reopen it, so I'll submit a new one.

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.

ls: enter_directory should use an iterative loop instead of recursing

2 participants