RFR: 4452735: Add GZIPOutputStream constructor to specify Deflater
Lance Andersen
lancea at openjdk.org
Fri Jan 24 20:39:50 UTC 2025
On Fri, 19 Jul 2024 15:16:01 GMT, Lance Andersen <lancea at openjdk.org> wrote:
>>> I understand the request here, but is there a current use case for needing a custom Deflater?
>>
>> I think the primary use case is when you want to set a non-default compression level, e.g., "best" or "fast". This is a pretty normal thing to do and matches what people expect from the `gzip(1)` command line flags. Allowing a custom `Deflater` is the simplest way to accomplish this; it also solves some other less common use cases, e.g., you want to set "no compression" for an already compressed file, or you want to keep the `Deflater` open so you can gather stats or whatever.
>>
>>> Before adding additional features, I think GZIP could benefit with more test coverage.
>>
>> Agreed. `GZIPOutputStream` does get some coverage in some of the `GZIPInputStream` tests, and this PR adds more testing, but certainly more is better.
>
>> > I understand the request here, but is there a current use case for needing a custom Deflater?
>>
>> I think the primary use case is when you want to set a non-default compression level, e.g., "best" or "fast". This is a pretty normal thing to do and matches what people expect from the `gzip(1)` command line flags. Allowing a custom `Deflater` is the simplest way to accomplish this; it also solves some other less common use cases, e.g., you want to set "no compression" for an already compressed file, or you want to keep the `Deflater` open so you can gather stats or whatever.
>
> thank you Archie. I don't have an issue with the feature request, but given this API has been around for since JDK 1.1 and there has not been a must have push for this enhancement, I would prefer to focus on JDK-8322256 closed out and adding more overall test coverage before tackling this.
>>
>> > Before adding additional features, I think GZIP could benefit with more test coverage.
>>
>> Agreed. `GZIPOutputStream` does get some coverage in some of the `GZIPInputStream` tests, and this PR adds more testing, but certainly more is better.
>
> Yes which why we should look to add additional tests, including more coverage from gzip files created via the gzip command line tool.
> @LanceAndersen, if you have time now I'd like to pick this one back up. A review for the CSR is also needed. Thanks for any comments.
I think it might be worth discussing the merits of whether other classes which extend DeflaterOutputStream should be given similar consideration and if consensus is reached should be addressed at the same time
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20226#issuecomment-2613339129
More information about the core-libs-dev
mailing list