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

Migrate to Uint8Array #410

Open
jimmywarting opened this issue Oct 23, 2022 · 6 comments
Open

Migrate to Uint8Array #410

jimmywarting opened this issue Oct 23, 2022 · 6 comments

Comments

@jimmywarting
Copy link

jimmywarting commented Oct 23, 2022

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

@mtth
Copy link
Owner

mtth commented Oct 31, 2022

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

@mtth mtth added the question label Oct 31, 2022
@jimmywarting
Copy link
Author

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: nodejs/node#39301 (comment)

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
Copy link
Author

jimmywarting commented Oct 31, 2022

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
Copy link
Author

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
Copy link
Owner

mtth commented Nov 2, 2022

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

@mtth mtth changed the title use less nodejs stuff Migrate to Uint8Array Nov 2, 2022
@mtth mtth added enhancement and removed question labels Nov 2, 2022
@jimmywarting
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants