perf!: reduce default brotli compression factor to 4, was before 11 (maximum)

[kahlstrm]

BREAKING CHANGE: The default brotli quality value currently is 11, which is not meant to be used in an on the fly compression context. Change this to a more reasonable default value of 4.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[gurgunday]

This looks like a pretty good change

Choosing anything higher than 6 has significant impact on the performance for little benefit

Ref: dotnet/runtime#26097 (comment)

[kahlstrm]

This looks like a pretty good change

Choosing anything higher than 6 has significant impact on the performance for little benefit

Ref: dotnet/runtime#26097 (comment)

In this case 4 was chosen as it's apparently a default used by Cloudflare. Related. Here level 5 provides some advantages at files above 64KB so that would also be an alternative.

[Uzlopak]

Yeah, makes sense. I also was avoiding brotli for the same reason, and preferred gzip as it has higher throughput. But maybe with brotli compression factor of 4 this drawback does not exist anymore.

[Uzlopak]

what does the title mean? sensible brotli value? Maybe some more distinctive title is good, so that reading the release notes it can be determined what changed.

perf: reduce default brotli compression factor to 4, was before 11 (maximum)

[gurgunday]

Will lgtm after the title change :)

[Uzlopak]

LGTM

[Fdawgs]

Maybe put an inline comment referencing why we've set the default to 4?

[Fdawgs]

Suggested change

	// Default of zlib is 11 which is way too heavy for on-the-fly, instead default to 4 as it's used by Cloudflare for on-the-fly compression https://blog.cloudflare.com/this-is-brotli-from-origin#testing
	/**
	* Default of 4 as 11 has a heavy impact on performance.
	* @see {@link https://blog.cloudflare.com/this-is-brotli-from-origin#testing}
	*/

[SimenB]

I'd argue changing the default is a breaking change, and not a "performance improvement". I agree it's a better default, but the behaviour shouldn't change without the consumer having the chance to decide if they value throughput over compression

[climba03003]

I'd argue changing the default is a breaking change

Yes, it is.

the behaviour shouldn't change without the consumer having the chance to decide if they value throughput over compression

@SimenB Maybe you overlook the code, it still allow to change the value in brotliOptions. The predefined one is before the spread operator which means it is not non-configurable.

[Uzlopak]

As it is breaking the api contract i also concur , that it is a semver-major.

[mcollina]

I'd argue changing the default is a breaking change, and not a "performance improvement".

I'm ok in bumping a major for this.

---

I agree it's a better default, but the behaviour shouldn't change without the consumer having the chance to decide if they value throughput over compression

Is this implemented already? https://github.com/fastify/fastify-compress?tab=readme-ov-file#brotlioptions-and-zliboptions

[Uzlopak]

@mcollina

Yes:

fastify-compress/index.js

Line 135 in 9e45ec9

br: () => ((opts.zlib || zlib).createBrotliCompress || zlib.createBrotliCompress)(params.brotliOptions),

[SimenB]

Yes, the user can set it (IIRC we did lower it at my previous workplace) - I'm just pointing out changing the default (even if configurable) is breaking 🙂 When I said "user should have the chance to decide" I merely meant a major version will "force" the user to make a conscious choice (as it'll be out of semver ranges for people setting that (most, if not all)), rather than a perf version, which is usually a patch release which I assume most people don't comb through changelogs for

[Uzlopak]

@SimenB
And to what value did you reduce it?

[SimenB]

I haven't worked there for 5 months, but I asked a former colleague, and he sent this snippet

// fastify-compress by default uses brotli quality at 11 (max), which is really
// slow. This sets it at 5 instead which is better for on-the-fly compression,
// based on https://blog.cloudflare.com/results-experimenting-brotli/
// Based on the same blog post, it sets zlib quality to 8.
// It also sets the operation to flush to work with streaming responses.
// Otherwise it will buffer everything (i.e. streaming won't work) which will
// impact TTFB.
const compressPlugin: FastifyPluginAsync = async instance => {
  instance.register(fastifyCompress, {
    zlibOptions: {
      flush: zlib.constants.Z_SYNC_FLUSH,
      level: 8,
    },
    brotliOptions: {
      flush: zlib.constants.BROTLI_OPERATION_FLUSH,
      params: {
        [zlib.constants.BROTLI_PARAM_QUALITY]: 5,
      },
    },
  });
};

(credit to @hzr)

[kahlstrm]

I haven't worked there for 5 months, but I asked a former colleague, and he sent this snippet

// fastify-compress by default uses brotli quality at 11 (max), which is really
// slow. This sets it at 5 instead which is better for on-the-fly compression,
// based on https://blog.cloudflare.com/results-experimenting-brotli/
// Based on the same blog post, it sets zlib quality to 8.
// It also sets the operation to flush to work with streaming responses.
// Otherwise it will buffer everything (i.e. streaming won't work) which will
// impact TTFB.
const compressPlugin: FastifyPluginAsync = async instance => {
  instance.register(fastifyCompress, {
    zlibOptions: {
      flush: zlib.constants.Z_SYNC_FLUSH,
      level: 8,
    },
    brotliOptions: {
      flush: zlib.constants.BROTLI_OPERATION_FLUSH,
      params: {
        [zlib.constants.BROTLI_PARAM_QUALITY]: 5,
      },
    },
  });
};

(credit to @hzr)

I've set mine to 5 as well in the project I'm using this in, so for me that's fine as well 👍 . The improvement of quality level 5 over 4 is seemingly more signifant with filesizes above 64 KB according to the Cloudflare article posted previously. Here is some analysis from 2015

[gurgunday]

8 seems too much 😅, but I fully get that it is much better than the current default

64kb is significant for text data, I don't believe the average size of an HTML page or a CSS file is up there

I think 4 is a good default

[kahlstrm]

8 seems too much 😅, but I fully get that it is much better than the current default

64kb is significant for text data, I don't believe the average size of an HTML page or a CSS file is up there

I think 4 is a good default

Note that in the snippet zlib is set at 8, which is comparable to brotli quality 4 IIRC. The brotli level inthe snippet is below to level 5.