High memory consumption when calling streaming call repeatedly

[pacu]

Hi! I'm Pacu! maintainer of Zcash iOS SDK. We've been using and
enjoying the library from quite some time now.

Describe the bug

Lately we changed the way we used GRPC calls from a long running stream (that could be hours downloading) to
sequential smaller calls, so we can download data and process it in parallel.

The problem with this is that we are now dealing with a huge chunk of small data structs being captured by GRPC inner workings and memory consumption spikes up heavily and quickly. I'm talking about obnoxious hundreds of MB in matter of seconds.

We are having problems figuring out whether this is a developer mistake from our side (us basically doing something wrong), "documentation bug" (API docs allow an undesired but yet permitted usage that causes the issue), or a grpc-swift (or any of its dependencies) bug.

grpc-swift versions: 1.15 and 1.16.0

What have we have tried so far

• We've reviewed that memory cycles are not being introduced on our side.
• We've reviewed all the documentation to make sure we are not messing up with unsupported calls
• We've updated to the latest version of the library.

To reproduce (sorry I don't have a sandboxed project to reproduce)

Steps to reproduce the bug you've found:

1. clone main of Zcash iOS SDK
2. run ZcashLightClientKitSample app on Xcode
3. run mainnet variant (for quicker results)
4. tap sync blocks
5. look at the debug pane of Xcode how memory spikes

after 10 seconds of sync

Something does not seem to be relinquishing memory

This is a stack trace of the allocs

Expected behaviour

I'd expect memory is released.

Additional information

This didn't happen when using a long running stream that told the server to stream a huge range of data. Only when we started to do subsequent smaller requests to the same streamer API

[Lukasa]

Based on looking at these images, these appear to be the backing storage objects of Data objects. These Data objects are produced as part of protobuf deserialization. Are your smaller calls still streaming calls?

Your use of AsyncThrowingStream(unfolding:) is a bit interesting to me here. That is capturing the iterator of the response stream on the streaming calls, and as long as it is alive we will not drop our response stream. Those response streams have interior buffers to maintain good throughput, so those buffers may be where the data is being stored. Are you holding onto the AsyncThrowingStreams anywhere in your code?

[pacu]

Does the stream drop buffers that have already been consumed? I'd say it does, maybe it isn't? or could it be the case that the stream is preemptively fetching too many elements because the server is faster sending us the data that us processing it?

We changed our code to refresh the stream every 1000 elements and we are seeing memory shape like waves.

[pacu]

we have sequential data from blockchain so we in this case are requesting blocks in ordinal sequence.

we in the issue we reported we were asking whichever was the range the user would need to catch up to, it could be a dozen or a million. if we requested range 0 ... 1,000,000 we would process small batches (from 10 to a 100) at a time and download another batch in the meanwhile.

if we request a whole range within a single stream 0...1_000_000 the memory will spike until your computer or device collapses.

If we request sub ranges of that range

 0…1000 
 1000…2000 
 2000…3000
 ...
 998000 ... 999000
 999000...1000000

we would see these "waves" of memory usage.

It seems that swift-grpc does have some caching that is handled internally

/// A high-low watermarked back-pressure strategy for a NIOAsyncSequenceProducer.
///
/// This strategy does the following:
/// - On yield it keeps on demanding more elements from the producer as long as the number of buffered elements hasn't reached the highWatermark.
/// - On consume it starts to demand again from the producer once the number of buffered elements reach the lowWatermark.

  let backpressureStrategy = NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark(
      lowWatermark: 10,
      highWatermark: 50
    )

we don't know how to tweak this maybe 50 means too much in our domain?

Also we used to handle this as a single call of and didn't have problems. I suspect that is back when we were not using async streams, but I'll have to do some work to be able to check and compare both APIs.

[pacu]

I was able to debug a bit more and I could debug a bit the watermark strategy on NIOThrowingAsyncSequenceProducer

/swift-nio/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift:881

I can see that the buffered count is 460 even though the strategy you folks are setting up is 10-50 elements. The buffer should never be more than 50 right?

[pacu]

Sorry for Spamming.

I've set a breakpoint inside NIO's didYield and it sometimes goes beyond the watermark like a 10% and others I can't determine because lldb is optimizing the values and I can't debug them but memory is tripling what the minimum consumption is, but when I'm able to get values from lldb with edited breakpoints they can go 6, 8 or 10x beyond high watermark.

ps: This issue seems to come and go sometimes.

[Lukasa]

Yeah, the high watermark is advisory, not a hard limit: beyond that watermark backpressure is supposed to be exerted on the producer, but the producer is entitled to continue to produce if it has stuff to do. This kind of "flexible" buffer is a common pattern in NIO-based code.

In this instance this appears to be some missing backpressure wiring, as discussed here:

grpc-swift/docs/async-await-proposal.md

Line 458 in 4ab02e1

### Backpressure

It's probably fairly important that we tackle wiring up the backpressure soon. @glbrntt any idea how substantial that work is? If @pacu was motivated enough to fix it himself, could he?

[glbrntt]

Unclear; I'll investigate and circle back.

[FranzBusch]

To add to what @Lukasa said there should be no memory leak at this point though. Once you drop the responseStream and its deinit gets called everything should get cleaned up.

[glbrntt]

I think there are quite a few changes required here. They aren't too complicated but they do touch a number of different places in the code and require a certain amount of care.

There are two 'high level' client calls which need updating:

• GRPCAsyncBidirectionalStreamingCall, and
• GRPCAsyncServerStreamingCall.

They are both provided with a Call on init where they also create a NIOThrowingAsyncSequenceProducer. The sequence producer has a delegate (currently GRPCAsyncSequenceProducerDelegate) with two methods:

• produceMore(), and
• didTerminate().

The GRPCAsyncSequenceProducerDelegate currently no-ops for both of these methods. For produceMore we'll need to add functionality to Call which eventually calls through to ClientTransport to ask it to produce more.

For didTerminate we should cancel the Call. Handily Call already has a cancel(promise:) function which we can reuse here.

The other part of this is to react to providing more elements to the Source of the NIOThrowingAsyncSequenceProducer. There are three outcomes:

• dropped,
• produceMore, and
• stopProducing.

For dropped we can simply do nothing. For produceMore we should call through to the same function on Call that we used in the delegate. For stopProducing we will need to add another function to Call (and ClientTransport) to tell it to stop reading.

This means that ClientTransport (which is also a NIO ChannelHandler) needs some more state management. It needs to know whether it's currently reading and whether it should continue to read. It will need to handle these state changes appropriately by issuing or stopping reads() to the Channel .

We should at some point also hook this up for the server but we can tackle that separately.

[lpbellier]

Is this issue still open?