-
Notifications
You must be signed in to change notification settings - Fork 654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Supports configurable compression level #3567
base: main
Are you sure you want to change the base?
Conversation
- gzip: only the range 0 to 9 is allowed - deflate: only the range 0 to 9 is allowed - zstd: only the range -7 to 22 is allowed brotli and snappy compression are supported by default. fix: reactor#3244
41bd91e
to
38f3bcc
Compare
SimpleCompressionHandler() { | ||
super((CompressionOptions[]) null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary constructors
@@ -1320,7 +1324,7 @@ static final class FailedHttpServerRequest extends HttpServerOperations { | |||
ZonedDateTime timestamp, | |||
ConnectionInfo connectionInfo, | |||
boolean validateHeaders) { | |||
super(c, listener, nettyRequest, null, connectionInfo, | |||
super(c, listener, nettyRequest, null, HttpCompressionSettingsSpec.provideDefault(), connectionInfo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifies the default compression level supported for each compression.
- gzip, deflate: 6
- zstd: 3
@@ -757,7 +761,7 @@ public HttpServerResponse compression(boolean compress) { | |||
removeHandler(NettyPipeline.CompressionHandler); | |||
} | |||
else if (channel().pipeline().get(NettyPipeline.CompressionHandler) == null) { | |||
SimpleCompressionHandler handler = new SimpleCompressionHandler(); | |||
SimpleCompressionHandler handler = SimpleCompressionHandler.create(compressionSettings); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applies the user-defined compression settings
to the Netty's compression settings.
public final HttpServer compressSettings(Consumer<HttpCompressionSettingsSpec.Builder> compressionSettings) { | ||
Objects.requireNonNull(compressionSettings, "compressionSettings"); | ||
|
||
HttpCompressionSettingsSpec.Builder builder = HttpCompressionSettingsSpec.builder(); | ||
compressionSettings.accept(builder); | ||
|
||
HttpServer dup = duplicate(); | ||
dup.configuration().compressionSettings = builder.build(); | ||
return dup; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method takes compression settings from the user.
return new Build(); | ||
} | ||
|
||
static HttpCompressionSettingsSpec provideDefault() { | ||
return new HttpCompressionSettingsSpec(); | ||
} | ||
|
||
CompressionOptions[] adaptToOptions() { | ||
List<CompressionOptions> options = new ArrayList<>(); | ||
options.add(this.gzipOptions); | ||
options.add(this.deflateOptions); | ||
options.add(this.snappyOptions); | ||
|
||
if (brotliOptions != null) { | ||
options.add(this.brotliOptions); | ||
} | ||
|
||
if (zstdOptions != null) { | ||
options.add(this.zstdOptions); | ||
} | ||
|
||
return options.toArray(new CompressionOptions[0]); | ||
} | ||
|
||
public interface Builder { | ||
|
||
/** | ||
* Build a new {@link HttpCompressionSettingsSpec}. | ||
* | ||
* @return a new {@link HttpCompressionSettingsSpec} | ||
*/ | ||
HttpCompressionSettingsSpec build(); | ||
|
||
/** | ||
* Sets the gzip compression level. | ||
* | ||
* @return a new {@link HttpCompressionSettingsSpec.Builder} | ||
*/ | ||
Builder gzip(int compressionLevel); | ||
|
||
/** | ||
* Sets the deflate compression level. | ||
* | ||
* @return a new {@link HttpCompressionSettingsSpec.Builder} | ||
*/ | ||
Builder deflate(int compressionLevel); | ||
|
||
/** | ||
* Sets the zstd compression level. | ||
* | ||
* @return a new {@link HttpCompressionSettingsSpec.Builder} | ||
*/ | ||
Builder zstd(int compressionLevel); | ||
} | ||
|
||
private static final class Build implements Builder { | ||
|
||
GzipOptions gzipOptions = StandardCompressionOptions.gzip(); | ||
DeflateOptions deflateOptions = StandardCompressionOptions.deflate(); | ||
ZstdOptions zstdOptions; | ||
|
||
private static final int DEFLATE_DEFAULT_WINDOW_BITS = 15; | ||
private static final int DEFLATE_DEFAULT_MEMORY_LEVEL = 8; | ||
private static final int ZSTD_DEFAULT_COMPRESSION_LEVEL = 3; | ||
private static final int ZSTD_DEFAULT_BLOCK_SIZE = 65536; | ||
private static final int ZSTD_MAX_BLOCK_SIZE = 1 << ZSTD_DEFAULT_COMPRESSION_LEVEL + 7 + 15; | ||
|
||
@Override | ||
public HttpCompressionSettingsSpec build() { | ||
return new HttpCompressionSettingsSpec(this); | ||
} | ||
|
||
@Override | ||
public Builder gzip(int compressionLevel) { | ||
ObjectUtil.checkInRange(compressionLevel, 0, 9, "compressionLevel"); | ||
|
||
gzipOptions = StandardCompressionOptions.gzip(compressionLevel, DEFLATE_DEFAULT_WINDOW_BITS, DEFLATE_DEFAULT_MEMORY_LEVEL); | ||
return this; | ||
} | ||
|
||
@Override | ||
public Builder deflate(int compressionLevel) { | ||
ObjectUtil.checkInRange(compressionLevel, 0, 9, "compressionLevel"); | ||
|
||
this.deflateOptions = StandardCompressionOptions.deflate(compressionLevel, DEFLATE_DEFAULT_WINDOW_BITS, DEFLATE_DEFAULT_MEMORY_LEVEL); | ||
return this; | ||
} | ||
|
||
@Override | ||
public Builder zstd(int compressionLevel) { | ||
if (!Zstd.isAvailable()) { | ||
throw new IllegalStateException("Unable to set compression level on zstd."); | ||
} | ||
ObjectUtil.checkInRange(compressionLevel, -7, 22, "compressionLevel"); | ||
|
||
this.zstdOptions = StandardCompressionOptions.zstd(compressionLevel, ZSTD_DEFAULT_BLOCK_SIZE, ZSTD_MAX_BLOCK_SIZE); | ||
return this; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defines a spec for configuring HTTP compression.
private HttpCompressionSettingsSpec() { | ||
gzipOptions = StandardCompressionOptions.gzip(); | ||
deflateOptions = StandardCompressionOptions.deflate(); | ||
snappyOptions = StandardCompressionOptions.snappy(); | ||
|
||
if (Brotli.isAvailable()) { | ||
brotliOptions = StandardCompressionOptions.brotli(); | ||
} | ||
|
||
if (Zstd.isAvailable()) { | ||
zstdOptions = StandardCompressionOptions.zstd(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default constructor
sets the available compression to default settings.
private HttpCompressionSettingsSpec(Build build) { | ||
gzipOptions = build.gzipOptions; | ||
deflateOptions = build.gzipOptions; | ||
snappyOptions = StandardCompressionOptions.snappy(); | ||
|
||
if (Brotli.isAvailable()) { | ||
brotliOptions = StandardCompressionOptions.brotli(); | ||
} | ||
|
||
if (Zstd.isAvailable() && build.zstdOptions != null) { | ||
zstdOptions = build.zstdOptions; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initializes to each compression setting entered by the Builder.
CompressionOptions[] adaptToOptions() { | ||
List<CompressionOptions> options = new ArrayList<>(); | ||
options.add(this.gzipOptions); | ||
options.add(this.deflateOptions); | ||
options.add(this.snappyOptions); | ||
|
||
if (brotliOptions != null) { | ||
options.add(this.brotliOptions); | ||
} | ||
|
||
if (zstdOptions != null) { | ||
options.add(this.zstdOptions); | ||
} | ||
|
||
return options.toArray(new CompressionOptions[0]); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adapts to the type to be provided to the constructor argument of Netty's HttpContentCompressor
.
private static final class Build implements Builder { | ||
|
||
GzipOptions gzipOptions = StandardCompressionOptions.gzip(); | ||
DeflateOptions deflateOptions = StandardCompressionOptions.deflate(); | ||
ZstdOptions zstdOptions; | ||
|
||
private static final int DEFLATE_DEFAULT_WINDOW_BITS = 15; | ||
private static final int DEFLATE_DEFAULT_MEMORY_LEVEL = 8; | ||
private static final int ZSTD_DEFAULT_COMPRESSION_LEVEL = 3; | ||
private static final int ZSTD_DEFAULT_BLOCK_SIZE = 65536; | ||
private static final int ZSTD_MAX_BLOCK_SIZE = 1 << ZSTD_DEFAULT_COMPRESSION_LEVEL + 7 + 15; | ||
|
||
@Override | ||
public HttpCompressionSettingsSpec build() { | ||
return new HttpCompressionSettingsSpec(this); | ||
} | ||
|
||
@Override | ||
public Builder gzip(int compressionLevel) { | ||
ObjectUtil.checkInRange(compressionLevel, 0, 9, "compressionLevel"); | ||
|
||
gzipOptions = StandardCompressionOptions.gzip(compressionLevel, DEFLATE_DEFAULT_WINDOW_BITS, DEFLATE_DEFAULT_MEMORY_LEVEL); | ||
return this; | ||
} | ||
|
||
@Override | ||
public Builder deflate(int compressionLevel) { | ||
ObjectUtil.checkInRange(compressionLevel, 0, 9, "compressionLevel"); | ||
|
||
this.deflateOptions = StandardCompressionOptions.deflate(compressionLevel, DEFLATE_DEFAULT_WINDOW_BITS, DEFLATE_DEFAULT_MEMORY_LEVEL); | ||
return this; | ||
} | ||
|
||
@Override | ||
public Builder zstd(int compressionLevel) { | ||
if (!Zstd.isAvailable()) { | ||
throw new IllegalStateException("Unable to set compression level on zstd."); | ||
} | ||
ObjectUtil.checkInRange(compressionLevel, -7, 22, "compressionLevel"); | ||
|
||
this.zstdOptions = StandardCompressionOptions.zstd(compressionLevel, ZSTD_DEFAULT_BLOCK_SIZE, ZSTD_MAX_BLOCK_SIZE); | ||
return this; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A builder that takes each compression setting as input from the user.
forwardedHeaderHandler, httpMessageLogFactory, listener, mapHandle, readTimeout, requestTimeout)); | ||
|
||
boolean alwaysCompress = compressPredicate == null && minCompressionSize == 0; | ||
|
||
if (alwaysCompress) { | ||
p.addLast(NettyPipeline.CompressionHandler, new SimpleCompressionHandler()); | ||
p.addLast(NettyPipeline.CompressionHandler, SimpleCompressionHandler.create(compressionSettings)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apply the configured compression settings.
* | ||
* @return a new {@link HttpCompressionSettingsSpec.Builder} | ||
*/ | ||
Builder gzip(int compressionLevel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raccoonback If in the future you need to extend the configuration API with window Bits
, memory level
etc. How will we introduce them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@violetagg
Thank you for the great question!
The reason why windowBits
and memoryLevel
are not currently configurable is that their default settings in GZIP are optimized for most use cases. Exposing these options could lead to unnecessary increases in memory usage or make configuration overly complex.
For now, we’ve chosen to rely on default values without exposing these settings directly to users. However, we’ve considered the need for extensibility in future requirements.
If it becomes necessary to make windowBits
and memoryLevel
configurable, the API can be extended using with method overloading:
Builder gzip(int compressionLevel);
Builder gzip(int compressionLevel, int windowBits);
Builder gzip(int compressionLevel, int windowBits, int memoryLevel);
// ...
Additionally, if required, we can extend the API to provide these as optional settings while maintaining the default values. Let me know your thoughts, and I’ll adjust the design accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm ... method overloading is not something that I would like to see in the API. Can we use a similar approach to Netty's CompressionOptions
. Thus the API will be "please provide CompressionOptions[]" and the API will stay stable and we can touch only particular CompressionOption
if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@violetagg
Thank you for your feedback!
I will revise the implementation to follow a structure similar to Netty's CompressionOptions
as you suggested.
- Updated the interface to accept `CompressionOptions[]` instead of using method overloading. - Aligns with Netty's design for consistency and maintainability, addressing feedback from PR reactor#3567.
final class BrotliOption implements HttpCompressionOption { | ||
|
||
private final CompressionOptions option; | ||
|
||
BrotliOption() { | ||
this.option = StandardCompressionOptions.brotli(); | ||
} | ||
|
||
CompressionOptions adapt() { | ||
return option; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brotli compression option
final class DeflateOption implements HttpCompressionOption { | ||
|
||
private final CompressionOptions option; | ||
|
||
DeflateOption() { | ||
this.option = StandardCompressionOptions.gzip(); | ||
} | ||
|
||
DeflateOption(int compressionLevel, int windowBits, int memoryLevel) { | ||
this.option = StandardCompressionOptions.gzip( | ||
compressionLevel, | ||
windowBits, | ||
memoryLevel | ||
); | ||
} | ||
|
||
CompressionOptions adapt() { | ||
return option; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deflate compression option
final class GzipOption implements HttpCompressionOption { | ||
|
||
private final CompressionOptions option; | ||
|
||
GzipOption() { | ||
this.option = StandardCompressionOptions.gzip(); | ||
} | ||
|
||
GzipOption(int compressionLevel, int windowBits, int memoryLevel) { | ||
this.option = StandardCompressionOptions.gzip( | ||
compressionLevel, | ||
windowBits, | ||
memoryLevel | ||
); | ||
} | ||
|
||
CompressionOptions adapt() { | ||
return option; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gzip compression option
private HttpCompressionOptionsSpec() { | ||
gzip = StandardHttpCompressionOptions.gzip(); | ||
deflate = StandardHttpCompressionOptions.deflate(); | ||
snappy = StandardHttpCompressionOptions.snappy(); | ||
|
||
if (Brotli.isAvailable()) { | ||
brotli = StandardHttpCompressionOptions.brotli(); | ||
} | ||
|
||
if (Zstd.isAvailable()) { | ||
zstd = StandardHttpCompressionOptions.zstd(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set all supported compression settings to their default settings, as before.
public HttpCompressionOptionsSpec(HttpCompressionOption... compressionOptions) { | ||
this(); | ||
Arrays.stream(compressionOptions) | ||
.forEach(this::initializeOption); | ||
} | ||
|
||
private void initializeOption(HttpCompressionOption option) { | ||
if (option instanceof GzipOption) { | ||
this.gzip = (GzipOption) option; | ||
} | ||
else if (option instanceof DeflateOption) { | ||
this.deflate = (DeflateOption) option; | ||
} | ||
else if (option instanceof SnappyOption) { | ||
this.snappy = (SnappyOption) option; | ||
} | ||
else if (Brotli.isAvailable() && option instanceof BrotliOption) { | ||
this.brotli = (BrotliOption) option; | ||
} | ||
else if (Zstd.isAvailable() && option instanceof ZstdOption) { | ||
this.zstd = (ZstdOption) option; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After setting the default compression settings, it overwrites the user's compression settings afterwards.
public CompressionOptions[] adapt() { | ||
List<CompressionOptions> options = new ArrayList<>( | ||
Arrays.asList( | ||
gzip.adapt(), | ||
deflate.adapt(), | ||
snappy.adapt() | ||
) | ||
); | ||
|
||
if (brotli != null) { | ||
options.add(brotli.adapt()); | ||
} | ||
|
||
if (zstd != null) { | ||
options.add(zstd.adapt()); | ||
} | ||
|
||
return options.toArray(new CompressionOptions[0]); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert this to compression settings to provide to Netty.
final class SnappyOption implements HttpCompressionOption { | ||
|
||
private final CompressionOptions option; | ||
|
||
SnappyOption() { | ||
this.option = StandardCompressionOptions.snappy(); | ||
} | ||
|
||
CompressionOptions adapt() { | ||
return option; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snappy compression option
} | ||
|
||
/** | ||
* Default GZIP options. | ||
* The default compression level is 6. | ||
* | ||
* @return a new {@link GzipOption} | ||
*/ | ||
public static GzipOption gzip() { | ||
return new GzipOption(); | ||
} | ||
|
||
/** | ||
* Set GZIP options. | ||
* | ||
* @return a new {@link GzipOption} | ||
* @throws IllegalStateException If invalid parameters. | ||
*/ | ||
public static GzipOption gzip(int compressionLevel, int windowBits, int memoryLevel) { | ||
ObjectUtil.checkInRange(compressionLevel, 0, 9, "compressionLevel"); | ||
ObjectUtil.checkInRange(windowBits, 9, 15, "windowBits"); | ||
ObjectUtil.checkInRange(memoryLevel, 1, 9, "memLevel"); | ||
|
||
return new GzipOption(compressionLevel, windowBits, memoryLevel); | ||
} | ||
|
||
/** | ||
* Default Deflate options. | ||
* The default compression level is 6. | ||
* | ||
* @return a new {@link DeflateOption} | ||
*/ | ||
public static DeflateOption deflate() { | ||
return new DeflateOption(); | ||
} | ||
|
||
/** | ||
* Set Deflate options. | ||
* | ||
* @return a new {@link DeflateOption} | ||
* @throws IllegalStateException If invalid parameters. | ||
*/ | ||
public static DeflateOption deflate(int compressionLevel, int windowBits, int memoryLevel) { | ||
ObjectUtil.checkInRange(compressionLevel, 0, 9, "compressionLevel"); | ||
ObjectUtil.checkInRange(windowBits, 9, 15, "windowBits"); | ||
ObjectUtil.checkInRange(memoryLevel, 1, 9, "memLevel"); | ||
|
||
return new DeflateOption(compressionLevel, windowBits, memoryLevel); | ||
} | ||
|
||
/** | ||
* Default ZSTD options. | ||
* The default compression level is 3. | ||
* | ||
* @return a new {@link ZstdOption} | ||
*/ | ||
public static ZstdOption zstd() { | ||
return new ZstdOption(); | ||
} | ||
|
||
/** | ||
* Set ZSTD options. | ||
* | ||
* @return a new {@link ZstdOption} | ||
* @throws IllegalStateException If invalid parameters. | ||
*/ | ||
public static ZstdOption zstd(int compressionLevel, int blockSize, int maxEncodeSize) { | ||
ObjectUtil.checkInRange(compressionLevel, -(1 << 17), 22, "compressionLevel"); | ||
ObjectUtil.checkPositive(blockSize, "blockSize"); | ||
ObjectUtil.checkPositive(maxEncodeSize, "maxEncodeSize"); | ||
|
||
if (!Zstd.isAvailable()) { | ||
throw new IllegalStateException("ZSTD is not available", Zstd.cause()); | ||
} | ||
|
||
return new ZstdOption(compressionLevel, blockSize, maxEncodeSize); | ||
} | ||
|
||
/** | ||
* Default Brotli options. | ||
* | ||
* @return a new {@link BrotliOption} | ||
*/ | ||
public static BrotliOption brotli() { | ||
if (!Brotli.isAvailable()) { | ||
throw new IllegalStateException("Brotli is not available", Brotli.cause()); | ||
} | ||
|
||
return new BrotliOption(); | ||
} | ||
|
||
/** | ||
* Default Snappy options. | ||
* | ||
* @return a new {@link SnappyOption} | ||
*/ | ||
public static SnappyOption snappy() { | ||
return new SnappyOption(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provides a method that allows users to set compression settings in a similar way to Netty.
final class ZstdOption implements HttpCompressionOption { | ||
|
||
private final CompressionOptions option; | ||
|
||
ZstdOption() { | ||
this.option = StandardCompressionOptions.zstd(); | ||
} | ||
|
||
ZstdOption(int compressionLevel, int windowBits, int memoryLevel) { | ||
this.option = StandardCompressionOptions.zstd( | ||
compressionLevel, | ||
windowBits, | ||
memoryLevel | ||
); | ||
} | ||
|
||
CompressionOptions adapt() { | ||
return option; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zstd compression option
@violetagg |
@@ -344,6 +346,7 @@ public Function<String, String> uriTagValue() { | |||
this.httpMessageLogFactory = ReactorNettyHttpMessageLogFactory.INSTANCE; | |||
this.maxKeepAliveRequests = -1; | |||
this.minCompressionSize = -1; | |||
this.compressionOptions = HttpCompressionOptionsSpec.provideDefault(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not initialise this with a default, or maybe it can be empty? The default will be used only when the compression is enabled. In Reactor Netty we try to not initialise things that will not be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@violetagg
Thank you for the suggestion!
I’ve updated the implementation in commit c28d954 to set compressionOptions to null as the default state when compression is disabled.
c28d954
to
3de0d82
Compare
…is disabled - Updated compressionOptions to default to null when compression is not explicitly configured. - Ensures a clean default state, reflecting that no compression settings are applied unless enabled.
3de0d82
to
7eee9c9
Compare
this.option = StandardCompressionOptions.gzip(); | ||
} | ||
|
||
DeflateOption(int compressionLevel, int windowBits, int memoryLevel) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagined some kind of a builder because I don't see how this is different than what was before. Let me play with the PR to see what we can achieve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback! 😃
In 2c653c1, I have implemented the compression settings to only be configurable via the methods provided by StandardHttpCompressionOptions
.
Before
HttpServer.create()
.compress(true)
.compressOptions(
builder -> builder.gzip(6)
.zstd(12)
)
.bindNow();
After
HttpServer.create()
.compress(true)
.compressOptions(
StandardHttpCompressionOptions.gzip(6, 15, 8),
StandardHttpCompressionOptions.zstd(12, 65536, 65536)
)
.bindNow();
By Builder approach, did you mean a method where each compression setting is handled through chainable calls, as shown below? 😊
HttpServer.create()
.compress(true)
.compressOptions(
GzipOptions.builder()
.compressionLevel(6)
.windowBits(15)
.windowBits(8)
.build(),
ZstdOptions.builder()
.compressionLevel(12)
.blockSize(65536)
.maxEncodeSize(65536)
.build()
)
.bindNow();
Yes, basically if you want to configure compression level, you configure
only this the rest stays the default and if in the future we need to add
something else then we just add only this new configuration to the builder
…On Fri, 10 Jan 2025 at 13:00, KOSEUNGBIN ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
reactor-netty-http/src/main/java/reactor/netty/http/server/compression/DeflateOption.java
<#3567 (comment)>
:
> +import io.netty.handler.codec.compression.StandardCompressionOptions;
+
+/**
+ * Deflate compression option configuration.
+ *
+ * @author raccoonback
+ */
+final class DeflateOption implements HttpCompressionOption {
+
+ private final CompressionOptions option;
+
+ DeflateOption() {
+ this.option = StandardCompressionOptions.gzip();
+ }
+
+ DeflateOption(int compressionLevel, int windowBits, int memoryLevel) {
Thank you for the feedback!
In this PR, I have implemented the compression settings to only be
configurable via the methods provided by StandardHttpCompressionOptions.
HttpServer.create()
.compress(true)
.compressOptions(
StandardHttpCompressionOptions.gzip(6, 15, 8),
StandardHttpCompressionOptions.zstd(12, 65536, 65536)
)
.bindNow();
By Builder approach, did you mean a method where each compression setting
is handled through chainable calls, as shown below? 😊
HttpServer.create()
.compress(true)
.compressOptions(
GzipOptions.builder()
.compressionLevel(6)
.windowBits(15)
.windowBits(8)
.build(),
ZstdOptions.builder()
.compressionLevel(12)
.blockSize(65536)
.maxEncodeSize(65536)
.build()
)
.bindNow();
—
Reply to this email directly, view it on GitHub
<#3567 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFKCVM43LJ5NXQ35WPOMGD2J6R4FAVCNFSM6AAAAABUKSI4SKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNBSGE2TKNZUGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@violetagg HttpServer.create()
.compress(true)
.compressOptions(
GzipOptions.builder()
.compressionLevel(6)
.windowBits(15)
.windowBits(8)
.build(),
ZstdOptions.builder()
.compressionLevel(12)
.blockSize(65536)
.maxEncodeSize(65536)
.build()
)
.bindNow(); |
- Refactor compression settings to use a Builder pattern for easier configuration and improved flexibility.
private int compressionLevel = 6; | ||
private int windowBits = 12; | ||
private int memoryLevel = 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set it the same as Netty's default GZip option.
private int compressionLevel = 3; | ||
private int blockSize = 1 << 16; // 64KB | ||
private int maxEncodeSize = 1 << (compressionLevel + 7 + 0x0F); // 32MB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Netty also uses the default values provided by zstd-jni for block size
and max encode size
.
I check in zstd-jni and set the default values.
private int compressionLevel = 6; | ||
private int windowBits = 12; | ||
private int memoryLevel = 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set it the same as Netty's default Deflate option.
@violetagg |
Motivation
Hello,
I have added a feature to support compression level configuration in Reactor Netty's
SimpleCompressionHandler
.Feature Details:
This enhancement is expected to enable users to better balance compression efficiency and performance.
Thanks.
Issue