Re: Proposal: Remove stopped transceivers to prevent resource exhaustion

Thanks Henrik for a good summary! I agree we should do something, but 
I'd caution against big changes. Comments inline.

On 2/15/19 6:02 AM, Henrik Boström wrote:
> Howdy WebRTC enthusiasts.
>
> Issue: Remove stopped transceivers from getTransceivers() 
> <https://github.com/w3c/webrtc-pc/issues/2092>
>
> TL;DR: Keeping transceivers around prevents garbage collection of a 
> possibly ever-growing list of tracks. Can we stop doing that?
>
> If an application does addTrack() and removeTrack() with 
> renegotiations in-between, the list of m= sections and transceivers 
> will grow over time.

This will remain true. Developers who operate with the 
addTrack()/removeTrack() symmetric mental model, will continue to 
experience this side-effect. transceiver.stop() does not rescue that 
mental model, since it stops both directions.

So the short answer to your "Can we stop doing that?" question is 
"probably not".

That said, I agree we should fix the main side-effect of 
transceiver.stop() you point out: that it holds on to objects long after 
the underlying machinery it's supposed to represent (the network 
negotiation slot) has been repurposed. Transceivers were meant to be 
true to the underlying behaviors, and this fails that.

> RTCRtpTransceiver.stop() allows negotiating that an m= section is no 
> longer used, and in subsequent renegotiations that m= section could be 
> recycled for future transceivers preventing the SDP from growing 
> indefinitely. However, currently the set of transceivers 
> <https://w3c.github.io/webrtc-pc/#transceivers-set> (getTransceivers()) 
> is never cleaned up and will grow indefinitely.
>
> This prevents the transceivers, and thus the associated 
> senders/receivers and tracks, from being garbage collected. A stress 
> test running Chrome that did add/removeTrack a lot ran out of memory 
> when we switched from Plan B to Unified Plan due to this resulting in 
> a few thousand transceivers in less than a minute. (I don't know the 
> true footprint, but even if each transceiver just has a single video 
> frame in its buffer of 1280x720x4, that's several megabytes, 
> multiplied by thousands of transceivers => gigabytes of data).
>
> *Shall we make stopping a transceiver remove it from the connection's 
> set of transceivers?*
>
> Note: It is still possible to prevent the list of transceivers from 
> growing by /manually/ recycling them using 
> transceiver.sender.replaceTrack() and transceiver.direction, but that 
> still wastes resources on transceivers currently not used, and implies 
> you probably shouldn't use transceiver.stop() in most cases.
>
> Related issues/questions:
>
>  1. If you care about mids of stopped transceivers you'd have to
>     record them yourself.
>  2. There is no "onstopped" event in case a transceiver is stopped due
>     to setRemoteDescription(). Should we add one, or is it enough that
>     the application checks whether the transceiver still exists by
>     querying getTransceivers() in track.onmute?
>
`mute` only fires remotely AFAIK. However, transceiver.stop() [1] does 
reliably fire `ended` on transceiver.receiver.track on both ends. [2] 
But in neither case are you given a pointer to the transceiver.

> Please discuss.

I think you hit on the most confusing situation: where the other side 
stopped your transceiver.

The current spec API for this is to examine the boolean 
`transceiver.stopped`, once you return to "stable". I think that works, 
has a couple of years of this working group behind it, Firefox 
implements it, and it has neither of the issues you mention.

With the change you propose, the transceiver would effectively disappear 
from all lists we maintain for developers, before developers get a 
chance to react. Finding out which transceiver(s) were stopped might be 
hard. Ironically, the `transceiver.stopped` API would still exist, but 
developers would have to cache the list ahead of time to use it.

How about instead:

  * we remove the stopped transceiver on subsequent renegotiation, when
    its m-line is reused?—If we're clever, we may even be able to insert
    the new transceiver that inherits the m-line, in its place,
    preserving the index order of the other transceivers in
    getTransceivers().

Thoughts?

.: Jan-Ivar :.

[1] http://w3c.github.io/webrtc-pc/#dom-rtcrtptransceiver-stop
[2] https://blog.mozilla.org/webrtc/rtcrtptransceiver-explored/

Received on Friday, 15 February 2019 18:05:18 UTC