diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index bbc62463b1e..48d91964772 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -1522,9 +1522,18 @@ private static unsafe int GetCurrentDirectoryWin32(int nBufferLength, char* lpBu [SupportedOSPlatform("windows")] internal static unsafe string GetFullPath(string path) { - int bufferSize = GetFullPathWin32(path, 0, null, IntPtr.Zero); - char* buffer = stackalloc char[bufferSize]; - int fullPathLength = GetFullPathWin32(path, bufferSize, buffer, IntPtr.Zero); + char* buffer = stackalloc char[MAX_PATH]; + int fullPathLength = GetFullPathWin32(path, MAX_PATH, buffer, IntPtr.Zero); + + // if user is using long paths we could need to allocate a larger buffer + if (fullPathLength > MAX_PATH) + { + char* newBuffer = stackalloc char[fullPathLength]; + fullPathLength = GetFullPathWin32(path, fullPathLength, newBuffer, IntPtr.Zero); + + buffer = newBuffer; + } + // Avoid creating new strings unnecessarily return AreStringsEqual(buffer, fullPathLength, path) ? path : new string(buffer, startIndex: 0, length: fullPathLength); } diff --git a/src/Tasks.UnitTests/Copy_Tests.cs b/src/Tasks.UnitTests/Copy_Tests.cs index 26e53c59436..d79ac579d3d 100644 --- a/src/Tasks.UnitTests/Copy_Tests.cs +++ b/src/Tasks.UnitTests/Copy_Tests.cs @@ -79,11 +79,6 @@ public static IEnumerable GetNullAndEmptyArrays() => */ }; - private const int NoParallelismThreadCount = 1; - private const int DefaultParallelismThreadCount = int.MaxValue; - - private int GetParallelismThreadCount(bool isUseSingleThreadedCopy) => isUseSingleThreadedCopy ? NoParallelismThreadCount : DefaultParallelismThreadCount; - /// /// Temporarily save off the value of MSBUILDALWAYSOVERWRITEREADONLYFILES, so that we can run /// the tests isolated from the current state of the environment, but put it back how it belongs @@ -509,7 +504,7 @@ public void DontCopyOverSameFile(bool isUseHardLinks, bool isUseSymbolicLinks, b UseSymboliclinksIfPossible = isUseSymbolicLinks, }; - t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy)); + t.Execute(m.CopyFile, !isUseSingleThreadedCopy); // Expect for there to have been no copies. Assert.Equal(0, m.copyCount); @@ -557,7 +552,7 @@ public void QuestionCopyFile(bool isUseHardLinks, bool isUseSymbolicLinks, bool FailIfNotIncremental = true, }; - Assert.False(t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy))); + Assert.False(t.Execute(m.CopyFile, !isUseSingleThreadedCopy)); // Expect for there to have been no copies. Assert.Equal(0, m.copyCount); @@ -617,7 +612,7 @@ public void QuestionCopyFileSameContent(bool isUseHardLinks, bool isUseSymbolicL SkipUnchangedFiles = true, FailIfNotIncremental = true, }; - Assert.True(t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy))); + Assert.True(t.Execute(m.CopyFile, !isUseSingleThreadedCopy)); // Expect for there to have been no copies. Assert.Equal(0, m.copyCount); @@ -670,7 +665,7 @@ public void QuestionCopyFileNotSameContent(bool isUseHardLinks, bool isUseSymbol FailIfNotIncremental = true, }; - Assert.False(t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy))); + Assert.False(t.Execute(m.CopyFile, !isUseSingleThreadedCopy)); // Expect for there to have been no copies. Assert.Equal(0, m.copyCount); @@ -2015,7 +2010,7 @@ public void CopyWithDuplicatesUsingFolder(bool isUseHardLinks, bool isUseSymboli filesActuallyCopied.Add(new KeyValuePair(source, dest)); } return true; - }, GetParallelismThreadCount(isUseSingleThreadedCopy)); + }, !isUseSingleThreadedCopy); Assert.True(success); Assert.Equal(2, filesActuallyCopied.Count); @@ -2082,7 +2077,7 @@ public void CopyWithDuplicatesUsingFiles(bool isUseHardLinks, bool isUseSymbolic filesActuallyCopied.Add(new KeyValuePair(source, dest)); } return true; - }, GetParallelismThreadCount(isUseSingleThreadedCopy)); + }, !isUseSingleThreadedCopy); Assert.True(success); Assert.Equal(4, filesActuallyCopied.Count); @@ -2352,7 +2347,7 @@ public void FailureWithNoRetries(bool isUseHardLinks, bool isUseSymbolicLinks, b }; var copyFunctor = new CopyFunctor(2, false /* do not throw on failure */); - bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy)); + bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy); Assert.False(result); engine.AssertLogDoesntContain("MSB3026"); @@ -2419,7 +2414,7 @@ public void SuccessAfterOneRetry(bool isUseHardLinks, bool isUseSymbolicLinks, b }; var copyFunctor = new CopyFunctor(2, false /* do not throw on failure */); - bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy)); + bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy); Assert.True(result); engine.AssertLogContains("MSB3026"); @@ -2446,7 +2441,7 @@ public void SuccessAfterOneRetryContinueToNextFile(bool isUseHardLinks, bool isU }; var copyFunctor = new CopyFunctor(2, false /* do not throw on failure */); - bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy)); + bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy); Assert.True(result); engine.AssertLogContains("MSB3026"); @@ -2478,7 +2473,7 @@ public void TooFewRetriesReturnsFalse(bool isUseHardLinks, bool isUseSymbolicLin }; var copyFunctor = new CopyFunctor(4, false /* do not throw */); - bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy)); + bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy); Assert.False(result); engine.AssertLogContains("MSB3026"); @@ -2507,7 +2502,7 @@ public void TooFewRetriesThrows(bool isUseHardLinks, bool isUseSymbolicLinks, bo }; var copyFunctor = new CopyFunctor(3, true /* throw */); - bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy)); + bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy); Assert.False(result); engine.AssertLogContains("MSB3026"); diff --git a/src/Tasks/Copy.cs b/src/Tasks/Copy.cs index e553d1765af..3cf26dc3eb4 100644 --- a/src/Tasks/Copy.cs +++ b/src/Tasks/Copy.cs @@ -15,6 +15,8 @@ using Microsoft.Build.Shared.FileSystem; using Microsoft.Build.Utilities; +using TPLTask = System.Threading.Tasks.Task; + #nullable disable namespace Microsoft.Build.Tasks @@ -44,6 +46,33 @@ public class Copy : TaskExtension, IIncrementalTask, ICancelableTask // taking up the whole threadpool esp. when hosted in Visual Studio. IOW we use a specific number // instead of int.MaxValue. private static readonly int DefaultCopyParallelism = NativeMethodsShared.GetLogicalCoreCount() > 4 ? 6 : 4; + private static Thread[] copyThreads; + private static AutoResetEvent[] copyThreadSignals; + private AutoResetEvent _signalCopyTasksCompleted; + + private static ConcurrentQueue _copyActionQueue = new ConcurrentQueue(); + + private static void InitializeCopyThreads() + { + lock (_copyActionQueue) + { + if (copyThreads == null) + { + copyThreadSignals = new AutoResetEvent[DefaultCopyParallelism]; + copyThreads = new Thread[DefaultCopyParallelism]; + for (int i = 0; i < copyThreads.Length; ++i) + { + AutoResetEvent autoResetEvent = new AutoResetEvent(false); + copyThreadSignals[i] = autoResetEvent; + Thread newThread = new Thread(ParallelCopyTask); + newThread.IsBackground = true; + newThread.Name = "Parallel Copy Thread"; + newThread.Start(autoResetEvent); + copyThreads[i] = newThread; + } + } + } + } /// /// Constructor. @@ -63,6 +92,8 @@ public Copy() RemovingReadOnlyAttribute = Log.GetResourceMessage("Copy.RemovingReadOnlyAttribute"); SymbolicLinkComment = Log.GetResourceMessage("Copy.SymbolicLinkComment"); } + + _signalCopyTasksCompleted = new AutoResetEvent(false); } private static string CreatesDirectory; @@ -79,7 +110,7 @@ public Copy() private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); // Bool is just a placeholder, we're mainly interested in a threadsafe key set. - private readonly ConcurrentDictionary _directoriesKnownToExist = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentDictionary _directoriesKnownToExist = new ConcurrentDictionary(DefaultCopyParallelism, DefaultCopyParallelism, StringComparer.OrdinalIgnoreCase); /// /// Force the copy to retry even when it hits ERROR_ACCESS_DENIED -- normally we wouldn't retry in this case since @@ -93,7 +124,7 @@ public Copy() /// private static readonly bool s_forceSymlinks = Environment.GetEnvironmentVariable("MSBuildUseSymboliclinksIfPossible") != null; - private static readonly int s_parallelism = GetParallelismFromEnvironment(); + private static readonly bool s_copyInParallel = GetParallelismFromEnvironment(); /// /// Default milliseconds to wait between necessary retries @@ -395,12 +426,12 @@ private void MakeFileWriteable(FileState file, bool logActivity) /// Copy the files. /// /// Delegate used to copy the files. - /// + /// /// Thread parallelism allowed during copies. 1 uses the original algorithm, >1 uses newer algorithm. /// internal bool Execute( CopyFileWithState copyFile, - int parallelism) + bool copyInParallel) { // If there are no source files then just return success. if (IsSourceSetEmpty()) @@ -430,9 +461,9 @@ internal bool Execute( try { - success = parallelism == 1 || DestinationFiles.Length == 1 + success = !copyInParallel || DestinationFiles.Length == 1 ? CopySingleThreaded(copyFile, out destinationFilesSuccessfullyCopied) - : CopyParallel(copyFile, parallelism, out destinationFilesSuccessfullyCopied); + : CopyParallel(copyFile, out destinationFilesSuccessfullyCopied); } catch (OperationCanceledException) { @@ -507,6 +538,22 @@ private bool CopySingleThreaded( return success; } + private static void ParallelCopyTask(object state) + { + AutoResetEvent autoResetEvent = (AutoResetEvent)state; + while (true) + { + if (_copyActionQueue.TryDequeue(out Action copyAction)) + { + copyAction(); + } + else + { + autoResetEvent.WaitOne(); + } + } + } + /// /// Parallelize I/O with the same semantics as the single-threaded copy method above. /// ResolveAssemblyReferences tends to generate longer and longer lists of files to send @@ -516,7 +563,6 @@ private bool CopySingleThreaded( /// private bool CopyParallel( CopyFileWithState copyFile, - int parallelism, out List destinationFilesSuccessfullyCopied) { bool success = true; @@ -559,77 +605,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, - CancellationToken = _cancellationTokenSource.Token - }; - var partitionCopyActionBlock = new ActionBlock>( - async (List partition) => - { - // Break from synchronous thread context of caller to get onto thread pool thread. - await System.Threading.Tasks.Task.Yield(); - for (int partitionIndex = 0; partitionIndex < partition.Count && !_cancellationTokenSource.IsCancellationRequested; partitionIndex++) - { - int fileIndex = partition[partitionIndex]; - ITaskItem sourceItem = SourceFiles[fileIndex]; - ITaskItem destItem = DestinationFiles[fileIndex]; - string sourcePath = sourceItem.ItemSpec; - - // Check if we just copied from this location to the destination, don't copy again. - MSBuildEventSource.Log.CopyUpToDateStart(destItem.ItemSpec); - bool copyComplete = partitionIndex > 0 && - String.Equals( - sourcePath, - SourceFiles[partition[partitionIndex - 1]].ItemSpec, - StringComparison.OrdinalIgnoreCase); - - if (!copyComplete) - { - if (DoCopyIfNecessary( - new FileState(sourceItem.ItemSpec), - new FileState(destItem.ItemSpec), - copyFile)) - { - copyComplete = true; - } - else - { - // Thread race to set outer variable but they race to set the same (false) value. - success = false; - } - } - else - { - MSBuildEventSource.Log.CopyUpToDateStop(destItem.ItemSpec, true); - } + ConcurrentQueue> partitionQueue = new ConcurrentQueue>(partitionsByDestination.Values); - if (copyComplete) - { - sourceItem.CopyMetadataTo(destItem); - successFlags[fileIndex] = (IntPtr)1; - } - } - }, - actionBlockOptions); + int activeCopyThreads = DefaultCopyParallelism; + for (int i = 0; i < DefaultCopyParallelism; ++i) + { + _copyActionQueue.Enqueue(ProcessPartition); + } + + InitializeCopyThreads(); - foreach (List partition in partitionsByDestination.Values) + for (int i = 0; i < DefaultCopyParallelism; ++i) { - bool partitionAccepted = partitionCopyActionBlock.Post(partition); - if (_cancellationTokenSource.IsCancellationRequested) - { - break; - } - else if (!partitionAccepted) - { - // Retail assert... - ErrorUtilities.ThrowInternalError("Failed posting a file copy to an ActionBlock. Should not happen with block at max int capacity."); - } + copyThreadSignals[i].Set(); } - partitionCopyActionBlock.Complete(); - partitionCopyActionBlock.Completion.GetAwaiter().GetResult(); + _signalCopyTasksCompleted.WaitOne(); // Assemble an in-order list of destination items that succeeded. destinationFilesSuccessfullyCopied = new List(DestinationFiles.Length); @@ -642,6 +634,65 @@ private bool CopyParallel( } return success; + + void ProcessPartition() + { + try + { + while (partitionQueue.TryDequeue(out List partition)) + { + for (int partitionIndex = 0; partitionIndex < partition.Count && !_cancellationTokenSource.IsCancellationRequested; partitionIndex++) + { + int fileIndex = partition[partitionIndex]; + ITaskItem sourceItem = SourceFiles[fileIndex]; + ITaskItem destItem = DestinationFiles[fileIndex]; + string sourcePath = sourceItem.ItemSpec; + + // Check if we just copied from this location to the destination, don't copy again. + MSBuildEventSource.Log.CopyUpToDateStart(destItem.ItemSpec); + bool copyComplete = partitionIndex > 0 && + String.Equals( + sourcePath, + SourceFiles[partition[partitionIndex - 1]].ItemSpec, + StringComparison.OrdinalIgnoreCase); + + if (!copyComplete) + { + if (DoCopyIfNecessary( + new FileState(sourceItem.ItemSpec), + new FileState(destItem.ItemSpec), + copyFile)) + { + copyComplete = true; + } + else + { + // Thread race to set outer variable but they race to set the same (false) value. + success = false; + } + } + else + { + MSBuildEventSource.Log.CopyUpToDateStop(destItem.ItemSpec, true); + } + + if (copyComplete) + { + sourceItem.CopyMetadataTo(destItem); + successFlags[fileIndex] = (IntPtr)1; + } + } + } + } + finally + { + int count = System.Threading.Interlocked.Decrement(ref activeCopyThreads); + if (count == 0) + { + _signalCopyTasksCompleted.Set(); + } + } + } } private bool IsSourceSetEmpty() @@ -1028,7 +1079,7 @@ private bool DoCopyWithRetries(FileState sourceFileState, FileState destinationF /// public override bool Execute() { - return Execute(CopyFileWithLogging, s_parallelism); + return Execute(CopyFileWithLogging, s_copyInParallel); } #endregion @@ -1049,18 +1100,10 @@ private static bool PathsAreIdentical(FileState source, FileState destination) return string.Equals(source.FileNameFullPath, destination.FileNameFullPath, FileUtilities.PathComparison); } - private static int GetParallelismFromEnvironment() + private static bool GetParallelismFromEnvironment() { int parallelism = Traits.Instance.CopyTaskParallelism; - if (parallelism < 0) - { - parallelism = DefaultCopyParallelism; - } - else if (parallelism == 0) - { - parallelism = int.MaxValue; - } - return parallelism; + return parallelism != 1; } } }