mtth / avsc

Avro for JavaScript :zap:
MIT License
1.29k stars 148 forks source link

Migrate to Uint8Array #410

Open jimmywarting opened 2 years ago

jimmywarting commented 2 years ago

wondering if you could switch to using say Uint8Array instead... \w helpers such as TextEncoder/Decoder/DataView instead

mtth commented 2 years ago

Hi @jimmywarting. No plan to do this for now; can you give some context on your use-case and how the change would help?

jimmywarting commented 2 years ago

I just consider Buffer to be quite bloated, it dose not exist in browsers or Deno so it isn't so cross comp friendly imo. You can accomplish pretty much the same thing by using Uint8array, DataView and textEncoder/decoder.

The buffer.toString() also dose not deal with BOM (which textEncoder dose) + the text_decoder that is uses is way slower in browsers: https://github.com/nodejs/node/issues/39301#issuecomment-877626152

And the buffer.slice() is broken and dose abnormal things.

There are quite a lot of ppl agreeing that Buffer is just bloated and unnecessary and it's bad for cross compatible code after they have discovered the true potential of plain Uint8Arrays... Buffer do also sometimes leak information in buf.buffer as its overflow with garbage

source:

jimmywarting commented 2 years ago

NodeJS streams are not so good either for cross compatible code. b/c it will will embed so much junk that you do not need. web streams are the new standard that exist now both in NodeJS, Deno and Browsers. But something better in my opinion is using a more low level (async)iterators that yields data instead and if you really need a NodeJS stream then you can use stream.Readable.from(iterator).pipe(...)

see my cross js guideline: https://github.com/cross-js/cross-js (some things might be a bit outdated)

jimmywarting commented 2 years ago

when i compared the size of avrc against other packers on https://jimmywarting.github.io/3th-party-structured-clone-wpt/

Then avrc came out on the top of the largest package (size is in gzip)

image

mtth commented 2 years ago

Thanks for the context.

avsc predates several of the changes which made Buffer alternatives competitive performance-wise, for example this DataView improvement which landed in Node 11. While migrating away from Buffer isn't planned for now, I would welcome a PR to do so (assuming it benchmarks favorably). Similarly for the streaming side with async iterators.

W.r.t. package size comparisons, you should use the serialization-only avsc distribution, avsc-types:

$ curl -s https://cdn.jsdelivr.net/npm/avsc@5.7.6/etc/browser/avsc-types.js/+esm|gzip|wc -c
23542
jimmywarting commented 2 years ago

Also the browser buffer haven't been updated in 2years and lagged behind in newer features

joscha commented 2 months ago

this was closed by https://github.com/mtth/avsc/pull/452 I think?

mtth commented 1 month ago

It will once 6.0 is released. Thinking of keeping this open until then.