Skip to content

Avoid throwing NotSupportedException in ReaderFactory hot path - #400

Merged
adamhathcock merged 1 commit into
adamhathcock:masterfrom
MattKotsenas:feature/avoid-exception-in-readerfactory
Jul 10, 2018
Merged

Avoid throwing NotSupportedException in ReaderFactory hot path#400
adamhathcock merged 1 commit into
adamhathcock:masterfrom
MattKotsenas:feature/avoid-exception-in-readerfactory

Conversation

@MattKotsenas

Copy link
Copy Markdown
Contributor

ReaderFactory.Open() calls ZipArchive.IsZipFile() to determine if the stream is a zip archive, which calls into ZipHeaderFactory.ReadHeader(), which throws a NotSupportedException when the bytes do not match a valid header.

To be clear, this exception is caught and IsZipFile() returns false, but when called in a hot-path, these exceptions can become expensive.

To address this issue, ReadHeader now returns null in the default cause instead of throwing. All callsites were already checking for and handling null, so no behavior should change.

`ReaderFactory.Open()` calls `ZipArchive.IsZipFile()` to determine if
the `Stream` is a zip archive, which calls into
`ZipHeaderFactory.ReadHeader()`, which throws a `NotSupportedException`
when the `Stream` is not a zip archive.

To be clear, this exception is caught and `IsZipFile()` returns `false`,
but when called in a hot-path, these exceptions can become expensive.

To address this issue, `ReadHeader` now returns `null` in the default
cause instead of throwing. All callsites were already checking for and
handling `null`, so no behavior changes.
@adamhathcock

Copy link
Copy Markdown
Owner

Thanks, this does make more sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants