[SignTool] Improve memory usage for large files when DryRun signing #15927
Conversation
| if (peHeaders.PEHeader.CheckSum == 0) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This is an unrelated fix but it allows dropping the memory usage further from 170MB -> 30MB when processing the android runtime pack since we avoid allocating the peBuffer (i.e. a complete copy of the .dll)
I'd appreciate a second set of eyes on this, from what I've seen the checksum shouldn't be zero when the file is signed, but I could be wrong.
There was a problem hiding this comment.
I think this is valid, based on lines 83-86 and the corresponding signing code. If the checksum was null, then for sure we would fail the checksum validation below. I wrote the sn verification to match the native sn.exe implementation.
| { | ||
| // the stream returned by zipArchiveEntry.Open() isn't seekable, | ||
| // we can avoid creating a MemoryStream copy by just opening a separate instance for computing the content hash | ||
| using var contentHashStream = zipArchiveEntry.Open(); |
|
I'll look at the test failures, they are related |
|
I took a quick look. On the surface, the tar.gz entries that are directory entries have a null content stream. But there are additional issues below that. |
We can avoid creating a MemoryStream copy of files in .nupkg by calling ZipArchiveEntry.Open() separately for computing the content hash.
This drops the memory usage from 1.1GB to 170MB (30MB with second fix, see below) when processing the android runtime pack.
Contributes to dotnet/dotnet#969