onBufferedAmountLow is annoying about datachannel-wasm HOT 11 CLOSED

The WebRTC bufferedamountlow event does not actually work that way, as there is no guarantee it will be called after send(). It should actually be called when bufferedAmount goes back under bufferedAmountLowThreshold (default 0), as the WebRTC specification says:

bufferedamountlow: The RTCDataChannel object's bufferedAmount decreases from above its bufferedAmountLowThreshold to less than or equal to its bufferedAmountLowThreshold.

So in the end, it boills down to how the browser calculates bufferedAmount. In practice, a browser is free to implement sending as it pleases, so it may not increment bufferedAmount if the message can be sent immediately, therefore send() won't necessarily result in a bufferedamountlow event (which is subject to change, see this bug for instance).

Therefore, the correct usage of the onBufferedAmountLow callback is as follows (provided bufferedAmountLowThreshold is 0):

• When a new message should be sent, put it in the queue and send queued messages until bufferedAmount() > 0.
• When the onBufferedAmountLow callback is called, send queued messages until bufferedAmount() > 0 (if the number of messages to send is big enough, you might want to stop at some point and schedule sending more a bit later so the browser event loop is not stuck for too long).

from datachannel-wasm.

@paullouisageneau

I understood your idea, but

When a new message should be sent, put it in the queue and send queued messages until bufferedAmount() > 0
When the onBufferedAmountLow callback is called, send queued messages until bufferedAmount() > 0

This sounds asyncish to me. When I want to send a new message, I should put it in queue, but when should I front and pop from queue?

Stackoverflow posts also had mentioned the difficulty of onBufferedAmountLow and they used setTimeout with specific ms to send messages and it looks like your idea goes same way.

from datachannel-wasm.

This sounds asyncish to me. When I want to send a new message, I should put it in queue, but when should I front and pop from queue?

What do you mean by asyncish? Of course the send method has to be non-blocking in the end, since you may not block the browser event loop.

You should pop from the queue both after you put a new message in the queue, and when onBufferedAmountLow is called. The pseudo code is rather simple:

queue<string> q;

void flush() {
    while (!q.empty() && dc->bufferedAmount() == 0) {
        dc->send(move(q.front()));
        q.pop();
    }
}

void send(string message) { // call this when you want to send a message
    q.push(move(message));
    flush();
}

dc->bufferedAmountLowThreshold(0);
dc->onBufferedAmountLow(flush);

Stackoverflow posts also had mentioned the difficulty of onBufferedAmountLow and they used setTimeout with specific ms to send messages and it looks like your idea goes same way.

The JavaScript API is awkwardly designed, but it's not that difficult. Indeed, setTimeout can be a useful refinement in the specific case you want to send a big and synchronous source of data as fast as possible.

In that case, there is no "a new message should be sent" situation, you always want to send and everything is virtually in the queue from the start. Therefore, the "send queued messages until bufferedAmount() > 0" loop might freeze the browser event loop for a while if the queue is big and bufferedAmount never increases because the throughput is very high.

A solution to this issue is to limit the maximum amount of data the loop can send in one call, and if it reaches the maximum, schedule it to be called again after a short delay, giving a chance to the event loop to run.

from datachannel-wasm.

According to your pseudo code, I don't see onBufferedAmountLow get chance to send any messages since the flush invocation of send method grab that chance.

from datachannel-wasm.

the "send queued messages until bufferedAmount() > 0" loop might freeze the browser event loop for a while if the queue is big and bufferedAmount never increases because the throughput is very high.

By the way, I'm sending 64k chunks so is this kind of throttling needed here?

from datachannel-wasm.

According to your pseudo code, I don't see onBufferedAmountLow get chance to send any messages since the flush invocation of send method grab that chance.

If dc->bufferedAmount() > 0, flush() will do nothing, an actual send will happen only when onBufferedAmountLow is called (it will be called when dc->bufferedAmount() == 0).

By the way, I'm sending 64k chunks so is this kind of throttling needed here?

It is irrelevant of the size of messages. It's needed only if your use case is sending a huge file as fast as possible.

from datachannel-wasm.

@paullouisageneau I used your code as-it is and with better variable names. I see that onBufferedAmountLow never get called and that only 1 message get out.

while(!messageQueue.empty() && dc && dc->bufferedAmount() > 0)

I suspect dc->bufferedAmount() > 0 might be causing this or dc->setBufferedAmountLowThreshold(0)

from datachannel-wasm.

onBufferedAmountLow get called if I send messages without your code. Just sending messages without queueing aka straightly.

from datachannel-wasm.

while(!messageQueue.empty() && dc && dc->bufferedAmount() > 0)

It should be dc->bufferedAmount() == 0, like in my example code, instead of dc->bufferedAmount() > 0.

from datachannel-wasm.

@paullouisageneau

It works now and feel somewhat more performant. Any idea why It become faster?

Closing.

from datachannel-wasm.

Any idea why It become faster?

Probably because using the callback properly like this ensures you send as soon as possible, so there is no idle time.

from datachannel-wasm.