perf(filesystem): optimize ListDirectory allocations and syscalls - #222
Draft
codspeed-hq[bot] wants to merge 1 commit into
Draft
Conversation
Contributor
There was a problem hiding this comment.
LGTM
The optimization is sound. Backing slice pointers are safe because fileBacking and subdirBacking are each capped at numEntries which is the total count, so neither can exceed capacity and trigger reallocation. The DT_UNKNOWN fallback is correctly handled. Permission formatting produces identical output.
Tag @mendral-app with feedback or questions. View session
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Optimizes
Filesystem.ListDirectory— invoked on every directory-listing HTTP request — by eliminating redundant syscalls and reducing heap allocations.Flamegraph analysis of
BenchmarkListDirectoryrevealed three independent sources of overhead beyond user/group lookups:1. Redundant
os.Lstatcalls for directory entriesos.ReadDirreturnsDirEntryvalues whoseIsDir()already knows the file type from thegetdentssyscall — no extra stat is needed. The original code calledos.Lstaton every entry, including directories. We now checkentry.IsDir()first and skipos.Lstatentirely for directories.os.Lstatis still used for files/symlinks (where full stat info is required), and a fallback handles the rareDT_UNKNOWNcase.2. Per-entry heap allocations causing GC pressure
Each
File/Subdirectorywas individually heap-allocated (&File{...}) and appended. For N entries this created N separate heap objects. We now pre-allocate backing slices ([]File,[]Subdirectory) sized tolen(entries)and take pointers into them, reducing N allocations to 2 and lowering GC scan work.3.
fmt.Sprintf("%o", ...)for permission formattingReplaced with
strconv.FormatUint(uint64(info.Mode()), 8), avoiding reflection and intermediate buffer allocation.Benchmark Results
Validated locally through the CodSpeed CLI (walltime):
BenchmarkListDirectoryNo regressions on other benchmarks. All existing filesystem and API tests pass.
Notes
Owner/group resolution for files is preserved via new
lookupUsername/lookupGroupnamehelpers, which resolve UID/GID from the already-obtainedsyscall.Stat_tand fall back to the numeric ID string on lookup failure — matching the prior behavior.Note
Optimizes
ListDirectoryby pre-allocating backing slices for File/Subdirectory structs, skippingos.Lstatfor directory entries (usingDirEntry.IsDir()fromgetdents), and replacingfmt.Sprintfwithstrconv.FormatUintfor permission formatting.Written by Mendral for commit ce9ce31.