Skip to content

Commit

Permalink
Restore: improve thread pool starvation fix (#2488)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dtivel
dtivel committed Oct 22, 2018
1 parent 288f942 commit 722c01e
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 61 deletions.
6 changes: 2 additions & 4 deletions src/NuGet.Core/NuGet.Packaging/LocalPackageArchiveDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,7 @@ public async Task<bool> CopyNupkgFileToAsync(string destinationFilePath, Cancell
FileMode.Create,
FileAccess.ReadWrite,
FileShare.ReadWrite | FileShare.Delete,
bufferSize: 4096,
useAsync: false))
bufferSize: 4096))
{
// This value comes from NuGet.Protocol.StreamExtensions.CopyToAsync(...).
// While 8K may or may not be the optimal buffer size for copy performance,
Expand Down Expand Up @@ -285,8 +284,7 @@ private FileStream GetSourceStream()
FileMode.Open,
FileAccess.Read,
FileShare.Read,
bufferSize: 4096,
useAsync: true);
bufferSize: 4096);
}

private void ThrowIfDisposed()
Expand Down
11 changes: 5 additions & 6 deletions src/NuGet.Core/NuGet.Packaging/PackageExtractor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,12 +458,11 @@ public static async Task<bool> InstallFromSourceAsync(
{
// Extract the nupkg
using (var nupkgStream = new FileStream(
targetTempNupkg,
FileMode.Create,
FileAccess.ReadWrite,
FileShare.ReadWrite | FileShare.Delete,
bufferSize: 4096,
useAsync: true))
targetTempNupkg,
FileMode.Create,
FileAccess.ReadWrite,
FileShare.ReadWrite | FileShare.Delete,
bufferSize: 4096))
{
await copyToAsync(nupkgStream);
nupkgStream.Seek(0, SeekOrigin.Begin);
Expand Down
40 changes: 9 additions & 31 deletions src/NuGet.Core/NuGet.Protocol/HttpSource/HttpCacheUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -67,44 +66,24 @@ public static async Task CreateCacheFileAsync(
// The update of a cached file is divided into two steps:
// 1) Delete the old file.
// 2) Create a new file with the same name.

// Some FileStream operations on Windows are synchronous even though it may not seem so.
// The HTTP stack rewrite in .NET Core 2.1 introduced circumstances whereby these
// synchronous FileStream calls will keep an IO completion thread busy and block other
// HTTP requests from completing. The immediate solution is to perform write and read
// operations on separate streams, but only on .NET Core where the problem exists.
// See https://github.com/dotnet/corefx/issues/31914 for details.
const int writeBufferSize =
#if IS_CORECLR
1; // This disables write buffering.
#else
BufferSize;
#endif

using (var fileStream = new FileStream(
result.NewFile,
FileMode.Create,
FileAccess.Write,
FileAccess.ReadWrite,
FileShare.None,
writeBufferSize,
useAsync: true))
BufferSize))
{
using (var networkStream = await response.Content.ReadAsStreamAsync())
{
await networkStream.CopyToAsync(fileStream, BufferSize, cancellationToken);
}
}

using (var fileStream = new FileStream(
result.NewFile,
FileMode.Open,
FileAccess.Read,
FileShare.None,
BufferSize,
useAsync: true))
{
// Validate the content before putting it into the cache.
ensureValidContents?.Invoke(fileStream);
if (ensureValidContents != null)
{
fileStream.Seek(0, SeekOrigin.Begin);
ensureValidContents.Invoke(fileStream);
}
}

if (File.Exists(result.CacheFile))
Expand Down Expand Up @@ -141,8 +120,7 @@ public static async Task CreateCacheFileAsync(
FileMode.Open,
FileAccess.Read,
FileShare.Read | FileShare.Delete,
BufferSize,
useAsync: true);
BufferSize);
}
}
}
}
5 changes: 2 additions & 3 deletions src/NuGet.Core/NuGet.Protocol/Plugins/PluginCacheEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ public async Task UpdateCacheFileAsync()
FileMode.Create,
FileAccess.ReadWrite,
FileShare.None,
CachingUtility.BufferSize,
useAsync: true))
CachingUtility.BufferSize))
{
var json = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(OperationClaims, Formatting.Indented));
await fileStream.WriteAsync(json, 0, json.Length);
Expand Down Expand Up @@ -116,4 +115,4 @@ public async Task UpdateCacheFileAsync()
}
}
}
}
}
6 changes: 2 additions & 4 deletions src/NuGet.Core/NuGet.Protocol/RemotePackageArchiveDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,7 @@ public async Task<bool> CopyNupkgFileToAsync(string destinationFilePath, Cancell
FileMode.Create,
FileAccess.ReadWrite,
FileShare.ReadWrite | FileShare.Delete,
bufferSize: 4096,
useAsync: true))
bufferSize: 4096))
{
var result = await _resource.CopyNupkgToStreamAsync(
_packageIdentity.Id,
Expand Down Expand Up @@ -286,8 +285,7 @@ private FileStream GetDestinationStream()
FileMode.Open,
FileAccess.Read,
FileShare.Read,
bufferSize: 4096,
useAsync: true);
bufferSize: 4096);
}

private void ThrowIfDisposed()
Expand Down
7 changes: 3 additions & 4 deletions src/NuGet.Core/NuGet.Protocol/Utility/CachingUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ public static Stream ReadCacheFile(TimeSpan maxAge, string cacheFile)
FileMode.Open,
FileAccess.Read,
FileShare.Read | FileShare.Delete,
BufferSize,
useAsync: true);
BufferSize);

return stream;
}
Expand All @@ -63,7 +62,7 @@ public static bool IsFileAlreadyOpen(string filePath)
{
stream = new FileStream(filePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
}
catch(FileNotFoundException)
catch (FileNotFoundException)
{
return false;
}
Expand Down Expand Up @@ -92,4 +91,4 @@ public static string RemoveInvalidFileNameChars(string value)
.Replace("__", "_");
}
}
}
}
13 changes: 6 additions & 7 deletions src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public async Task<NuspecReader> GetNuspecReaderFromNupkgAsync(
CancellationToken token)
{
NuspecReader reader = null;

lock (_nuspecReadersLock)
{
if (_nuspecReaders.TryGetValue(url, out reader))
Expand All @@ -76,7 +76,7 @@ await ProcessNupkgStreamAsync(
cacheContext,
logger,
token);

if (reader == null)
{
// The package was not found on the feed. This typically means
Expand Down Expand Up @@ -245,7 +245,7 @@ private async Task<CacheEntry> ProcessStreamAndGetCacheEntryAsync(
logger,
token);
}

private async Task<T> ProcessHttpSourceResultAsync<T>(
PackageIdentity identity,
string url,
Expand Down Expand Up @@ -336,8 +336,7 @@ private async Task<bool> ProcessCacheEntryAsync(
FileMode.Open,
FileAccess.Read,
FileShare.ReadWrite | FileShare.Delete,
StreamExtensions.BufferSize,
useAsync: true));
StreamExtensions.BufferSize));
},
token))
{
Expand All @@ -354,9 +353,9 @@ public CacheEntry(string cacheFile, bool alreadyProcessed)
CacheFile = cacheFile;
AlreadyProcessed = alreadyProcessed;
}

public string CacheFile { get; }
public bool AlreadyProcessed { get; }
}
}
}
}
4 changes: 2 additions & 2 deletions src/NuGet.Core/NuGet.Protocol/Utility/GetDownloadResultUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ private static async Task<DownloadResourceResult> DirectDownloadAsync(
FileAccess.ReadWrite,
FileShare.Read,
BufferSize,
FileOptions.Asynchronous | FileOptions.DeleteOnClose);
FileOptions.DeleteOnClose);

await packageStream.CopyToAsync(fileStream, BufferSize, token);

Expand All @@ -218,4 +218,4 @@ private static async Task<DownloadResourceResult> DirectDownloadAsync(
}
}
}
}
}

0 comments on commit 722c01e

Please sign in to comment.