Update copy logic to use dedicated threads. #11272
Conversation
|
Just kinda generally, I'm surprised to hear about threadpool starvation in our worker nodes which aren't threadpool heavy. Have you observed this in practice? |
There is some sneaky utilization of the threadpool that ends up blocking enough to see the "hill climbing" algorithm start spinning up new threads. I've seen between 40 and 50 threadpool threads per MSBuild processes which is over 3x what I would expect. This change (#11275) has a larger impact than the copy code, but this contributes as well. There's also additional benefit to reducing the mix of I/O bound work and CPU bound work on the threadpool. |
|
Related to #11160 |
Also updated some checks to help avoid hitting the file system if possible.
JanProvaznik
force-pushed
the
dev/erarndt/updateCopyParallelism
branch
from
d0500f7 to
868969a
Compare
| @@ -288,6 +319,7 @@ private void LogAlwaysRetryDiagnosticFromResources(string messageResourceName, p | |||
| } | |||
|
|
|||
| if (!Traits.Instance.EscapeHatches.CopyWithoutDelete && | |||
| (UseHardlinksIfPossible || UseSymboliclinksIfPossible) && | |||
There was a problem hiding this comment.
This seems to break tests. Why is it beneficial?
| @@ -559,77 +607,23 @@ private bool CopyParallel( | |||
|
|
|||
| // Lockless flags updated from each thread - each needs to be a processor word for atomicity. | |||
| var successFlags = new IntPtr[DestinationFiles.Length]; | |||
| var actionBlockOptions = new ExecutionDataflowBlockOptions | |||
| { | |||
| MaxDegreeOfParallelism = parallelism, | |||
There was a problem hiding this comment.
the PR lost the logic for parallelism variable values, this should be cleaned up to be transparent that it does nothing apart from deciding if execution is synchronous or parallel (with the default values).
|
Is there an option to use TaskCreationOptions.LongRunning flag? This should avoid the thread starvation problem. |
Also updated some checks to help avoid hitting the file system if possible.
Fixes #
Context
The existing implementation does synchronous file copying on threadpool threads which can lead to starvation. Switching to dedicated threads to do synchronous copying helps keep the threadpool threads available for other tasks.
Changes Made
Testing
Notes