Skip to content

Commit

Permalink
Code review feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
KirillOsenkov committed Jan 11, 2021
1 parent 69c860f commit f658232
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/Build/Logging/BinaryLogger/BinaryLogReplayEventSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ public void Replay(string sourceFilePath, CancellationToken cancellationToken)
using (var stream = new FileStream(sourceFilePath, FileMode.Open, FileAccess.Read, FileShare.Read))
{
var gzipStream = new GZipStream(stream, CompressionMode.Decompress, leaveOpen: true);

// wrapping the GZipStream in a buffered stream significantly improves performance
// and the max throughput is reached with a 32K buffer. See details here:
// https://github.com/dotnet/runtime/issues/39233#issuecomment-745598847
var bufferedStream = new BufferedStream(gzipStream, 32768);
var binaryReader = new BinaryReader(bufferedStream);

Expand Down
4 changes: 4 additions & 0 deletions src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,10 @@ public void Dispose()
}
catch
{
// The StringStorage class is not crucial for other functionality and if
// there are exceptions when closing the temp file, it's too late to do anything about it.
// Since we don't want to disrupt anything and the file is in the TEMP directory, it will
// get cleaned up at some point anyway.
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/Build/Microsoft.Build.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@
<!-- Do not generate a warning that our 'stable' package should not have a prerelease dependency. -->
<NoWarn>$(NoWarn);NU5104</NoWarn>

<!-- Fnv-1a 64-bit hash we use in BuildEventArgsWriter to deduplicate strings uses UInt64 which is not CLS-compliant -->
<NoWarn>$(NoWarn);CS3001;CS3002;CS3003</NoWarn>

</PropertyGroup>

<ItemGroup>
Expand Down

0 comments on commit f658232

Please sign in to comment.