Skip to content

Commit 9fa9d80

Browse files
authored
Update copy logic to use dedicated threads. (#11272)
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. Also updated some checks to help avoid hitting the file system if possible.
1 parent 79c777a commit 9fa9d80

File tree

3 files changed

+149
-102
lines changed

3 files changed

+149
-102
lines changed

src/Framework/NativeMethods.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,9 +1522,18 @@ private static unsafe int GetCurrentDirectoryWin32(int nBufferLength, char* lpBu
15221522
[SupportedOSPlatform("windows")]
15231523
internal static unsafe string GetFullPath(string path)
15241524
{
1525-
int bufferSize = GetFullPathWin32(path, 0, null, IntPtr.Zero);
1526-
char* buffer = stackalloc char[bufferSize];
1527-
int fullPathLength = GetFullPathWin32(path, bufferSize, buffer, IntPtr.Zero);
1525+
char* buffer = stackalloc char[MAX_PATH];
1526+
int fullPathLength = GetFullPathWin32(path, MAX_PATH, buffer, IntPtr.Zero);
1527+
1528+
// if user is using long paths we could need to allocate a larger buffer
1529+
if (fullPathLength > MAX_PATH)
1530+
{
1531+
char* newBuffer = stackalloc char[fullPathLength];
1532+
fullPathLength = GetFullPathWin32(path, fullPathLength, newBuffer, IntPtr.Zero);
1533+
1534+
buffer = newBuffer;
1535+
}
1536+
15281537
// Avoid creating new strings unnecessarily
15291538
return AreStringsEqual(buffer, fullPathLength, path) ? path : new string(buffer, startIndex: 0, length: fullPathLength);
15301539
}

src/Tasks.UnitTests/Copy_Tests.cs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,6 @@ public static IEnumerable<object[]> GetNullAndEmptyArrays() =>
7979
*/
8080
};
8181

82-
private const int NoParallelismThreadCount = 1;
83-
private const int DefaultParallelismThreadCount = int.MaxValue;
84-
85-
private int GetParallelismThreadCount(bool isUseSingleThreadedCopy) => isUseSingleThreadedCopy ? NoParallelismThreadCount : DefaultParallelismThreadCount;
86-
8782
/// <summary>
8883
/// Temporarily save off the value of MSBUILDALWAYSOVERWRITEREADONLYFILES, so that we can run
8984
/// 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
509504
UseSymboliclinksIfPossible = isUseSymbolicLinks,
510505
};
511506

512-
t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy));
507+
t.Execute(m.CopyFile, !isUseSingleThreadedCopy);
513508

514509
// Expect for there to have been no copies.
515510
Assert.Equal(0, m.copyCount);
@@ -557,7 +552,7 @@ public void QuestionCopyFile(bool isUseHardLinks, bool isUseSymbolicLinks, bool
557552
FailIfNotIncremental = true,
558553
};
559554

560-
Assert.False(t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy)));
555+
Assert.False(t.Execute(m.CopyFile, !isUseSingleThreadedCopy));
561556

562557
// Expect for there to have been no copies.
563558
Assert.Equal(0, m.copyCount);
@@ -617,7 +612,7 @@ public void QuestionCopyFileSameContent(bool isUseHardLinks, bool isUseSymbolicL
617612
SkipUnchangedFiles = true,
618613
FailIfNotIncremental = true,
619614
};
620-
Assert.True(t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy)));
615+
Assert.True(t.Execute(m.CopyFile, !isUseSingleThreadedCopy));
621616

622617
// Expect for there to have been no copies.
623618
Assert.Equal(0, m.copyCount);
@@ -670,7 +665,7 @@ public void QuestionCopyFileNotSameContent(bool isUseHardLinks, bool isUseSymbol
670665
FailIfNotIncremental = true,
671666
};
672667

673-
Assert.False(t.Execute(m.CopyFile, GetParallelismThreadCount(isUseSingleThreadedCopy)));
668+
Assert.False(t.Execute(m.CopyFile, !isUseSingleThreadedCopy));
674669

675670
// Expect for there to have been no copies.
676671
Assert.Equal(0, m.copyCount);
@@ -2015,7 +2010,7 @@ public void CopyWithDuplicatesUsingFolder(bool isUseHardLinks, bool isUseSymboli
20152010
filesActuallyCopied.Add(new KeyValuePair<FileState, FileState>(source, dest));
20162011
}
20172012
return true;
2018-
}, GetParallelismThreadCount(isUseSingleThreadedCopy));
2013+
}, !isUseSingleThreadedCopy);
20192014

20202015
Assert.True(success);
20212016
Assert.Equal(2, filesActuallyCopied.Count);
@@ -2082,7 +2077,7 @@ public void CopyWithDuplicatesUsingFiles(bool isUseHardLinks, bool isUseSymbolic
20822077
filesActuallyCopied.Add(new KeyValuePair<FileState, FileState>(source, dest));
20832078
}
20842079
return true;
2085-
}, GetParallelismThreadCount(isUseSingleThreadedCopy));
2080+
}, !isUseSingleThreadedCopy);
20862081

20872082
Assert.True(success);
20882083
Assert.Equal(4, filesActuallyCopied.Count);
@@ -2352,7 +2347,7 @@ public void FailureWithNoRetries(bool isUseHardLinks, bool isUseSymbolicLinks, b
23522347
};
23532348

23542349
var copyFunctor = new CopyFunctor(2, false /* do not throw on failure */);
2355-
bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy));
2350+
bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy);
23562351

23572352
Assert.False(result);
23582353
engine.AssertLogDoesntContain("MSB3026");
@@ -2419,7 +2414,7 @@ public void SuccessAfterOneRetry(bool isUseHardLinks, bool isUseSymbolicLinks, b
24192414
};
24202415

24212416
var copyFunctor = new CopyFunctor(2, false /* do not throw on failure */);
2422-
bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy));
2417+
bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy);
24232418

24242419
Assert.True(result);
24252420
engine.AssertLogContains("MSB3026");
@@ -2446,7 +2441,7 @@ public void SuccessAfterOneRetryContinueToNextFile(bool isUseHardLinks, bool isU
24462441
};
24472442

24482443
var copyFunctor = new CopyFunctor(2, false /* do not throw on failure */);
2449-
bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy));
2444+
bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy);
24502445

24512446
Assert.True(result);
24522447
engine.AssertLogContains("MSB3026");
@@ -2478,7 +2473,7 @@ public void TooFewRetriesReturnsFalse(bool isUseHardLinks, bool isUseSymbolicLin
24782473
};
24792474

24802475
var copyFunctor = new CopyFunctor(4, false /* do not throw */);
2481-
bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy));
2476+
bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy);
24822477

24832478
Assert.False(result);
24842479
engine.AssertLogContains("MSB3026");
@@ -2507,7 +2502,7 @@ public void TooFewRetriesThrows(bool isUseHardLinks, bool isUseSymbolicLinks, bo
25072502
};
25082503

25092504
var copyFunctor = new CopyFunctor(3, true /* throw */);
2510-
bool result = t.Execute(copyFunctor.Copy, GetParallelismThreadCount(isUseSingleThreadedCopy));
2505+
bool result = t.Execute(copyFunctor.Copy, !isUseSingleThreadedCopy);
25112506

25122507
Assert.False(result);
25132508
engine.AssertLogContains("MSB3026");

0 commit comments

Comments
 (0)