Add back managed implementation of System.IO.Compression

[weshaggard]

We want to put System.IO.Compression into NetStandard.Library (not NetStandard.Platform) but in order to do that we need to have a fallback implementation that is pure IL that can be used on new platforms that may not have the native dependency supported.

cc @ericstj @KrzysztofCwalina

[stephentoub]

Why does that require "putting it back"? Wouldn't that just be a different build of the binary? In which case this could be done if/when such a platform arises?

[weshaggard]

@stephentoub part of the criteria for adding something to NetStandard.Library (StdLib) is that it must be pure IL and can run on top of any .NET Platform that implements NetStandard.Platform (StdPlat). That doesn't prelude it from having more efficient native dependencies on some platforms it is more about future proofing the library so it just works on any new platforms.

However after looking at System.IO.Compression's dependencies I see it also now depends on System.Buffers which isn't part of StdPlat or StdLib and we don't want to include it in either at this time so I'm not planning to include System.IO.Compression in StdLib yet. For that reason I just pushed this to Future and we can consider doing the work at a future point in time.

[ianhays]

The System.Buffers is a relatively new addition that, like the zlib dependency, is there for perf reasons. The library could certainly function without either of those things, but it would come with a performance hit.

I'd like to know more about the stdlib thing. Aren't we trying to avoid having our own implementations for things that are available natively? And Zlib is available pretty much everywhere.

[stephentoub]

part of the criteria for adding something to NetStandard.Library (StdLib) is that it must be pure IL and can run on top of any .NET Platform that implements NetStandard.Platform (StdPlat).

Understood. Maybe this is a question of phrasing. I have no qualms about a pure-managed implementation. But when you said "fallback", I thought you meant in the same way it was there previously, I.e. try at runtime to invoke the native lib, and if that fails, use a managed implemention. I don't see why we'd want to do that, especially when we already effectively have multiple builds for multiple platforms and different versions of the underlying lib. I'm fine if we want to bring back the managed implementation as yet another separate build that can be deployed for platforms that lack zlib, but a) it's much more likely that zlib will be available than coreclr being available, b) there's no need to do this until such a hypothetical platform emerges, and c) until such a time it'd be a large chunk of dead code, which would likely rot. To me, the criteria shouldn't be whether a purely managed implementation is available but whether it can be implemented purely as a managed component, such that such an implementation can be provided if/when needed. Maybe we're saying the same thing.

[weshaggard]

@stephentoub I think we are mostly on the same page.

I'm fine if we want to bring back the managed implementation as yet another separate build

Yes this is what I was requesting.

a) it's much more likely that zlib will be available than coreclr being available

We have to think larger scope then coreclr and consider any new .NET runtime that might be on any new OS or processor.

b) there's no need to do this until such a hypothetical platform emerges,

For our general packages I agree we can wait but for StdLib we want to make a stronger statement which is more along the lines of it will just work without us needing to ship a new package as long as the new platform provides an implementation for StdPlat.

c) until such a time it'd be a large chunk of dead code, which would likely rot.

This is a fair concern which I do agree with but if we did do it we would need to keep tests running on it to keep it from rotting.

At any rate right now I'm not proposing we do anything more System.IO.Compression yet because I'm not planning to add it to StdLib yet.

@ianhays feel free to find me and I can chat more with you about StdLib.

[ianhays]

One of my main concerns with this is that the managed implementation is buggy and inefficient and terribly dated. Before we switched to using zlib, Mark Adler frequently advocated using a third party library instead of our Compression. That's a pretty bad sign when the creator of gzip recommends against your implementation of gzip.

That said, I see the benefits of having platform-agnostic compression capability and am fine with adding back the managed code as a separate build so we have some sort of default/fallback/backup, at least until we can get a clrcompression.dll ready on that platform. As long as we're not relying upon the managed implementation for anything but edge cases, it will be fine.

We also will need to worry about future additions like https://github.com/dotnet/corefx/issues/7570 that will introduce code drift between the version of System.IO.Compression using zlib and the version using a managed implementation. I imagine we're not going to want to put in man-hours adding new functionality to what is essentially a legacy build (somewhat ironically used for future platforms), so what will we do in that scenario? PlatformNotSupportedExceptions?

However after looking at System.IO.Compression's dependencies I see it also now depends on System.Buffers which isn't part of StdPlat or StdLib and we don't want to include it in either at this time so I'm not planning to include System.IO.Compression in StdLib yet.

@stephentoub removed all of the Buffers usage in dotnet/corefx@908c81d, so that isn't a blocker.

[ianhays]

In dotnet/corefx#11264 I added back the managed implementation as an internal class: DeflateManagedStream.

It would be fairly easy to now add a conditionally included (whenever we're on an unknown platform) DeflateStream.cs that wraps DeflateManagedStream . That would make Compression able to run on any platform, but there's a question of how we want to test it then to prevent it getting more stale than it already is.