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

Fetch highWaterMark #923

Open
ronag opened this issue Aug 4, 2021 · 10 comments
Open

Fetch highWaterMark #923

ronag opened this issue Aug 4, 2021 · 10 comments
Labels
enhancement New feature or request fetch
Milestone

Comments

@ronag
Copy link
Member

ronag commented Aug 4, 2021

What should we use as highWaterMark for the fetch api? I think browsers usually have 1m+ while Node has ~16k. Should we try matching the browsers? I think this has some implications on functionality when using Response.clone.

@ronag ronag added bug Something isn't working fetch labels Aug 4, 2021
@szmarczak
Copy link
Member

szmarczak commented Aug 4, 2021

Can it be updated dynamically? E.g when the response is not cloned, then it can be 64kb. If someone reads from the clone then increase it again by 64kb if it's been read.

@ronag
Copy link
Member Author

ronag commented Aug 4, 2021

I guess it could.

@ronag ronag added enhancement New feature or request and removed bug Something isn't working labels Aug 4, 2021
@ronag ronag mentioned this issue Aug 4, 2021
39 tasks
@ronag
Copy link
Member Author

ronag commented Aug 4, 2021

But if it's only 64k I think we could just have it as a default. It's more relevant if we have something like > 128k.

@ronag
Copy link
Member Author

ronag commented Aug 4, 2021

@szmarczak
Copy link
Member

But if it's only 64k I think we could just have it as a default. It's more relevant if we have something like > 128k.

Performance-wise I don't think there's a difference.

because we are not reading from cloned stream at the time, its internal buffer will fill up, eventually, causing both stream to pause and hence what we called backpressure.

I mean if the response is cloned, then every time it's consumed bit by bit increase the highWaterMark so it can flow.

Example:

  1. A new response is created.
  2. It's being cloned.
  3. 64k is read from the clone. In order not to pause the stream, increase the highWaterMark by 64k.
  4. The highWaterMark is now 128k.
  5. Another 64k is read from the clone. In order not to pause the stream, increase the highWaterMark by 64kb.
  6. The highWaterMark is now 192k.
  7. 192k is read from the original response. In order not to pause the stream, increase highWaterMark again by 64k.
  8. The highWaterMark is now 256k.

@ronag
Copy link
Member Author

ronag commented Aug 4, 2021

We need a "max" highWaterMark then.

@szmarczak
Copy link
Member

Ah. Then I think 1MB should be more than enough.

@ronag
Copy link
Member Author

ronag commented Aug 4, 2021

I wonder if it is possible to implement a custom stream controller? @jasnell

@mcollina
Copy link
Member

mcollina commented Aug 4, 2021

Take a look at https://www.npmjs.com/package/cloneable-readable on how I clone Node.js streams. In our world I did that by setting the highwaterMark of the clone, and then the pipe mechanism did the rest.

@ronag
Copy link
Member Author

ronag commented Aug 4, 2021

This is unfortunately for web streams, not node streams.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fetch
Projects
None yet
Development

No branches or pull requests

3 participants