perf: raise MaxDegreeOfParallelism in ResolveDirectoryTree - #3836
Open
Mpdreamz wants to merge 2 commits into
Open
perf: raise MaxDegreeOfParallelism in ResolveDirectoryTree#3836Mpdreamz wants to merge 2 commits into
Mpdreamz wants to merge 2 commits into
Conversation
MinimalParseAsync is ~60% blocked in the open() syscall (~755µs/file across 435k files). With the default ProcessorCount cap the IO queue stays shallow; Thread.StartCallback showing 805s own time in dotTrace confirms pool workers are parked waiting for IO to complete. Two fixes in one commit: 1. Materialise MarkdownFiles (FrozenSet<T>) into an array before the parallel loop. FrozenSet is not IList<T>, so Parallel.ForEachAsync falls back to the locked-enumerator partitioner rather than range partitioning — extra lock contention at high DOP. 2. Set MaxDegreeOfParallelism = max(ProcessorCount * 4, 32). The 4x multiplier keeps enough IO in flight to cover the latency; capped at 32 to avoid runaway RSS (each in-flight parse holds a MarkdownDocument tree in memory). The right multiplier should be re-validated with bench on the target machine once the other perf PRs land. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Parallel.ForEachAsync (async) does not use Partitioner.Create and has no IList<T> fast path — it locks a shared enumerator regardless of whether the source is a FrozenSet or a flat array. The array copy allocated 435k references into a second data structure for no gain. The actual fix is solely the MaxDegreeOfParallelism change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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
MinimalParseAsyncis ~60% blocked in theopen()syscall (~755µs/file across 435k files). With the defaultProcessorCountcap the IO queue stays shallow —Thread.StartCallbackshowing 805s own time in dotTrace confirms pool workers are parked waiting for IO to complete.MarkdownFilesis aFrozenSet<MarkdownFile>which is notIList<T>, soParallel.ForEachAsyncfalls back to the locked-enumerator partitioner rather than range partitioning — extra lock contention at high DOP.Two fixes:
MarkdownFilesinto an array before the loop so the partitioner gets an indexable source.MaxDegreeOfParallelism = max(ProcessorCount × 4, 32). The 4× multiplier keeps enough IO in flight to cover the latency; the 32 floor avoids stalling on machines with very few cores.The multiplier should be re-validated with
docs-migrate benchon the target machine. Each in-flight parse holds aMarkdownDocumentin memory, so there is a real wall-time vs RSS trade-off.Test plan
dotnet test tests/Elastic.Markdown.Tests/— 1951 tests passdotnet run --project src/tooling/docs-migrate -- bench— recordResolveDirectoryTreetime before and after🤖 Generated with Claude Code