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

Memory growth when grpc-web streaming #511

Open
gawmanarnar opened this issue Aug 4, 2021 · 3 comments
Open

Memory growth when grpc-web streaming #511

gawmanarnar opened this issue Aug 4, 2021 · 3 comments

Comments

@gawmanarnar
Copy link
Contributor

I currently have an application that streams frames over grpc to flutter mobile and web clients. When running in mobile, the memory seems stable but when doing it in web it appears the data coming from GRPC never gets garbage collected and memory just continues to grow (until the browser runs out of memory). The entirety of these messages are GC'd when the stream is closed but I need to keep the stream open (live video). Is there a way to release these messages as they are processed? I'm wondering if I'm missing something simple. I know grpc-web is done over http but surely this is possible?

Note: the code I wrote is basically the same between platforms and it uses the GrpcOrGrpcWebClientChannel.

Version information:
url: "https://github.com/grpc/grpc-dart.git"
ref: f23070e
version: "3.0.1-dev"

Repro steps

  1. Have a GRPC server capable of streaming data to a client
  2. Create a GrpcOrGrpcWebClientChannel (running on web)
  3. Call getStream on it
  4. Add a listener to process data

Expected result: Data is garbage collected after being processed

Actual result: Data sticks around and causes memory to grow until out of memory.

Details

Memory from Mobile (looks reasonable)
image

Memory from Web (constantly increasing)
image

@mraleph
Copy link
Member

mraleph commented Aug 9, 2021

Unfortunately that's an expected behaviour from the current implementation.

Currently gRPC-Web protocol implementation builds on top of XHR API which does not actually have any concept of streaming - instead protocol parser simply slices the tail of the XHR response as new bytes arrive (see the code here: https://github.com/grpc/grpc-dart/blob/master/lib/src/client/transport/xhr_transport.dart#L80-L92), which means the browser is slowly accumulating more and more data in memory (the concatenation of all streaming responses), because it thinks that the whole response to XHR is needed and there is no way to discard already processed chunks.

To address this memory leak one would need to reimplement gRPC-Web transport on top of a streaming friendly Fetch API (or WebSocket API - though WS have their own problems as far as I know).

@jbrownsw
Copy link

jbrownsw commented May 1, 2022

So I'm working on something for this and it's mostly working, but certain calls seem to be failing for some reason. There's actually hundreds of calls that are working before I get to these few that are failing. The problem seems to be that I'm creating a body string of 70 characters, but for whatever reason fetch is putting a content-length of 72 in the header so the response I get back is unexpected eof which obviously makes sense because I'm sending 70 bytes and they're expecting 72.

I'm just using String.fromCharCodes() to encode the data, but I'm no expert on dart/javascript interop so maybe there's a better option here. If I just let the List go through it encodes it as just a comma separated list of numbers and that fails. I was just wondering if I could get some feedback since this is hopefully the last problem I need to wrap up before I can submit a PR:
https://github.com/jbrownsw/grpc-dart/blob/master/lib/src/client/transport/fetch_transport.dart

I used chrome developer tools and copied the request as a fetch request from xhr and my fetch code and they're both completely identical except for the content-length. An example of the data in question is:

NativeUint8List ([0, 0, 0, 0, 65, 10, 59, 81, 117, 103, 110, 87, 117, 50, 67, 49, 114, 50, 73, 88, 57, 107, 54, 56, 118, 106, 65, 73, 65, 58, 52, 100, 101, 48, 55, 48, 55, 50, 45, 97, 51, 54, 54, 45, 52, 49, 51, 48, 45, 98, 101, 98, 54, 45, 97, 55, 50, 55, 98, 101, 55, 55, 51, 100, 102, 54, 16, 128, 130, 104])

Note that it was modeled after xhr_transport with FetchHttpRequest meant to be mostly a drop in replacement for HttpRequest so they could basically share the same code by the time it's ready.

@jbrownsw
Copy link

jbrownsw commented May 1, 2022

Ok I have a better understanding of the problem after looking at it a bit more. I think the issue is that the dev tools only show me the body value as a string rather than in bytes. It's the bytes that are >=128 that result in more than 1 byte and the mismatching content-length which I assume is related to the encoding javascript uses for strings.

The data really needs to be Uint8Array.from($data) however whenever that gets marshalled back to dart it just takes the type NativeUint8List which is what we had to begin with. It feels like a bit of a bug that marshalling from javascript to dart results in that, but marshaling from dart to javascript just results in a comma separated string of ints. My current working proof concept I have locally is using javascript eval to get around this, but I don't feel like that's a good solution so I'm still looking for ways to properly keep it in Uint8Array form when passing it to body.

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

3 participants