whatwg / compression

Compression Standard
https://compression.spec.whatwg.org/
Other
82 stars 21 forks source link

Simplify example. Fixes #29. #30

Closed jakearchibald closed 4 years ago

ricea commented 4 years ago

I don't know... it's certainly more concise, but it's hiding a lot of performance cost. I'm worried about sucking people into a slow pattern. I will try to do some actual measurements and get back to you.

sindresorhus commented 4 years ago

Show both then. With the simple one first.

jakearchibald commented 4 years ago

If the performance is very different, maybe we need some higher level APIs:

function compressArrayBuffer(input) {
  return ReadableStream.from(input)
    .pipeThrough(new CompressionStream('deflate'))
    .arrayBuffer();
}
ricea commented 4 years ago

If the performance is very different, maybe we need some higher level APIs:

function compressArrayBuffer(input) {
  return ReadableStream.from(input)
    .pipeThrough(new CompressionStream('deflate'))
    .arrayBuffer();
}

Yes, this is a combination of https://github.com/whatwg/streams/issues/1018 and https://github.com/whatwg/streams/issues/1019.

ricea commented 4 years ago

For compressing 50KB, the version using Blob is over 10 times slower in Chrome 80.0.3987.106:

Compressing 52101 bytes of data 10000 times with compressArrayBufferLong
compressArrayBufferLong: 810.84423828125ms
Compressing 52101 bytes of data 10000 times with compressArrayBufferShort
compressArrayBufferShort: 10981.843017578125ms

Could you retain the existing implementation under the new implementation, marked "(verbose but high-performance version)" or similar?

jakearchibald commented 4 years ago

Whoa, any idea where the overhead comes from? The conversion to blob, or to response?

jakearchibald commented 4 years ago

Makes me wonder if https://github.com/wicg/compression/blob/master/explainer.md#gzip-decompress-a-blob-to-a-blob is similarly slow, and makes even more of a case for the high-level methods.

ricea commented 4 years ago

Whoa, any idea where the overhead comes from? The conversion to blob, or to response?

Blob, because it involves the browser process. If you use the Response constructor to convert-to-stream instead, it's only twice as slow.

ricea commented 4 years ago

Makes me wonder if https://github.com/wicg/compression/blob/master/explainer.md#gzip-decompress-a-blob-to-a-blob is similarly slow, and makes even more of a case for the high-level methods.

There's an implicit assumption in this example that you're using a Blob because your data is huge. Maybe we should add an explicit note that this will perform poorly for "small" data.

jakearchibald commented 4 years ago

Interesting! I've changed the PR to use Response rather than Blob.

ricea commented 4 years ago

Thanks. I'd still like to add some guidance in how to do it performantly, but I can do that in a follow-up change.

jakearchibald commented 4 years ago

Thanks for looking into the performance characteristics! I learned a lot.

jimmywarting commented 4 years ago

Hi 👋 I'm just curious whats make new blob([x]).stream() slower than new response(x).body 😕

ricea commented 4 years ago

Hi I'm just curious whats make new blob([x]).stream() slower than new response(x).body

See https://bugs.chromium.org/p/chromium/issues/detail?id=1053486#c1 for some background.