Skip to content
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

Brotli Defaults #121

Closed
niftylettuce opened this issue Jul 6, 2020 · 13 comments
Closed

Brotli Defaults #121

niftylettuce opened this issue Jul 6, 2020 · 13 comments

Comments

@niftylettuce
Copy link
Contributor

@uhop curious your opinion on this - I had to do the following to make compression work fast for https://forwardemail.net:

https://github.com/ladjs/web/blob/6c2c09e5f78f403194883d169a4823dec73da743/index.js#L152-L159

@jonathanong
Copy link
Member

better default values would be cool, at least in the readme

@niftylettuce
Copy link
Contributor Author

niftylettuce commented Jul 6, 2020

I think we should have a v6.0.0 release with default Brotli encoding options. Otherwise I am guessing that 99% of use cases in Koa will suffer from the poor performance due to Node defaults. Where/how might I put in default values for br @jonathanong @uhop ?

Edit: People also don't always read the README, they just install and plug in the middleware and think it has sensible defaults. The Node defaults aren't sensible enough for Koa so probably should set our own.

@uhop
Copy link
Contributor

uhop commented Jul 6, 2020

@niftylettuce Looks good to me. Some notes:

  • You can play with a window size as well. Bigger size may give you better compression without too much slowdown.
  • The mode is "text". Obviously it depends on what you serve the most. The default is "generic".

This brings us to a scenario when we may want to fine-tune compression depending on a type of payload, or its size. If it would be up to me, I would implement it using my two favorite techniques:

  1. Functional global settings. Allow using a function as settings. Example of the idea (pseudo-code):

    // user's code
    const app = new Koa()
    app.use(compress({
      br: (type, size) => {
        // we can be as selective as we can:
        if (/^image\//i.test(type)) return null;
        if (/^text\//i.test(type) || type === 'application/json') return {
          [zlib.constants.BROTLI_PARAM_MODE]: zlib.constants.BROTLI_MODE_TEXT,
          [zlib.constants.BROTLI_PARAM_QUALITY]: 4
        }
        // e.g., some compressible non-text payload
        return { [zlib.constants.BROTLI_PARAM_QUALITY]: 4 }
      }
    }))

    And the change in koajs/compress would be minimal:

    let encodingOptions = options[encoding]
    while (typeof encodingOptions == 'function') encodingOptions = encodingOptions(type, size);
    // now we can continue like before (index.js, L79):
    const stream = ctx.body = compress(encodingOptions)
    // and so on

    Two extra lines buy a lot of flexibility without introducing new options nor any complexity on the user's side and on the implementor's side.

    This technique can be used for all options. It doesn't cost much in terms of code size or CPU.

  2. Flexible individual settings. We already have a boolean value compress, which can be set on ctx to indicate if we want any compression at all. It can be reinterpreted as truthy/falsy values (totally compatible with what we have now), and if we want to compress, we return an object, which can be mixed to global compression options. A sketch:

    // user's code:
    app.use((ctx, next) => {
      // ...
      // used to be (and still works): ctx.compress = true
      ctx.compress = { br: { [zlib.constants.BROTLI_PARAM_QUALITY]: 8 } } 
      ctx.body = fs.createReadStream(file)
    })

    Again the change in koajs/compress would be minimal:

    // inside the processing loop
    // options is a global compression options, uncracked yet
    if (ctx.compress === false) return // bailing out
    const responseOptions = {...options, ...ctx.compress}
    let { filter = compressible, threshold = 1024, defaultEncoding = 'identity' } = responseOptions
    // and so on

These techniques bring the following benefits:

  • Totally compatible with the existing user's code. Honestly, it is just refinement.
  • Utmost flexibility on the user's side. Do you want X? Go and implement it. Be as fine-grained as you want to be.
  • Minimal code changes. No code bloat. Easy to maintain.
  • Minimal documentation change. Very low cognitive price for users and for maintainers.

@niftylettuce
Copy link
Contributor Author

@uhop this would be incredible. might you be able to open a PR for it?

@uhop
Copy link
Contributor

uhop commented Jul 6, 2020

Will do tomorrow.

@damianaiamad
Copy link

damianaiamad commented Sep 2, 2020

I agree that koa-compress should not use the default brotli compression level (11). The performance is just terrible.

For comparison, with a 12MB json file (real world data):

BROTLI
Level 1 Compression 88.7% Time 36ms
Level 2 Compression 89.3% Time 72ms
Level 3 Compression 89.6% Time 74ms
Level 4 Compression 91.1% Time 112ms
Level 5 Compression 92.0% Time 180ms
Level 6 Compression 92.1% Time 222ms
Level 7 Compression 92.2% Time 302ms
Level 8 Compression 92.3% Time 397ms
Level 9 Compression 92.4% Time 545ms
Level 10 Compression 93.1% Time 6441ms
Level 11 Compression 93.6% Time 33956ms <<<< NodeJS default

GZIP
Level 1 Compression 87.4% Time 75ms
Level 2 Compression 87.8% Time 79ms
Level 3 Compression 88.2% Time 91ms
Level 4 Compression 89.1% Time 115ms
Level 5 Compression 89.4% Time 135ms
Level 6 Compression 89.9% Time 169ms
Level 7 Compression 90.0% Time 215ms
Level 8 Compression 90.2% Time 505ms
Level 9 Compression 90.3% Time 557ms

From this very limited test, one might select L9 as a reasonable default. Performance slows by 10x by selecting L10!

@uhop
Copy link
Contributor

uhop commented Sep 2, 2020

In prod I use 5 or 7 (I don't recall how or why I arrived to these values). According to your tests, it is possible to do as well as gzip-9 with br-4 yet be faster than gzip-4.

@damianaiamad
Copy link

damianaiamad commented Sep 2, 2020

5 is a sensible setting. I plotted Level against compression % and time. The results are informative.

Obviously results will differ with different data. The sample here is a 3MB JSON file.

Note: Level 10 & 11 go way off the end of the chart. I just ignore them as not useful for realtime compression.

image

image

@damianaiamad
Copy link

damianaiamad commented Sep 2, 2020

And just to make the point how slow NodeJS's default brotliCompress() is for realtime compression, here is the same chart with the time axis rescaled so that you can see how far up it goes! Level 11 is NodeJS default for brotli.

Ouch!

image

@damianaiamad
Copy link

@uhop I like your suggested changes to allow more flexibility.

I agree with @niftylettuce that it would be a good idea for koa-compress to specify it's own default for brotli, to give reasonable out of the box performance.

I'm thinking Brotli 5 may be the best all-round default. You could also make an argument for Brotli 9.

More analysis here.
https://blogs.akamai.com/2016/02/understanding-brotlis-potential.html

@jonathanong
Copy link
Member

i'm thinking a encodingDefaults object around here: https://github.com/koajs/compress/blob/master/lib/encodings.js#L18

that gets deep-merged with options on initialization

@joepio
Copy link

joepio commented Jun 25, 2021

The current default compression leads to absolutely terrible performance. This default high compression option just made our initial page load 10x longer then necessary (we have a 1.2mb resource that includes the content for the entire website). Setting the compression level to 7 decreased initial page load from 4.6 secs to 0.4s.

patrickhulce added a commit to patrickhulce/compress that referenced this issue Dec 16, 2021
3d205cd didn't actually fix the default compression issue described in koajs#121 because the zlib format expects quality to be inside [an option called `params`](https://nodejs.org/api/zlib.html#class-brotlioptions)
titanism pushed a commit that referenced this issue Jan 26, 2023
* fix: correct brotli options format

3d205cd didn't actually fix the default compression issue described in #121 because the zlib format expects quality to be inside [an option called `params`](https://nodejs.org/api/zlib.html#class-brotlioptions)

* tests: update defaults test

* chore: remove trailing whitespace
@mrtcmn
Copy link

mrtcmn commented Aug 1, 2023

For future viewers, maybe any params it's not suitable for you. Because brotli compression ratio is depend your data.

This little snippet may usefull for understanding, which settings is best.

https://gist.github.com/mrtcmn/6f8301a41c26cffaaccbf7ed078c7221

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

No branches or pull requests

6 participants