Do not IO Block and waste million of CPU cycles

[thanpolas]

following up on this specific part of the code

There is a method named Temasys.WebRTCPlugin.WaitForPluginReady whose job is to turn JS into IO Blocking by entering in an infinite loop.

Forcibly turning an event driven language into IO Blocking is a failure to realize that this is an async operation.

You are bringing thousands of machines to their knees, wasting million of CPU cycles because you fail to grasp how this is an async operation and how to handle it. The very nature of JS, being a single threaded language, should not even allow Temasys.WebRTCPlugin.isPluginReady to ever become true, you got away with pure luck here.

Considering you are wildy distributing this code, I strongly suggest you refactor this and all the methods (they are plenty) that invoke it.

[johache]

Hi Thanpolas,

First of all, thank you for helping us improving our product. We are always happy to receive contructive critique from users. However, I would point that a little bit of curtesy usually helps making the resolution of an issue faster;

Secondly, if you have a better implementation to offer - which I don't at the moment - , I suggest you submit a PR, and we will review it as soon as possible

Best regards, J-O

[serrynaimo]

Hey @thanpolas,

what we struggle with (and why we waste loads of CPU cycles) is, how to make a JS constructor function yield without doing this ridiculous loop while remaining compatible back to IE8. Do you have an idea?

[thanpolas]

@johache I tried courtesy in my original comment, on the commit, but it didn't get through, so I had to be blunt.

This is a fundamental core design issue that you need to look into, it's not something any outsider developer can submit a PR for.

Most of the [WebRTC] normalized methods that use the WaitForPluginReady() method have callbacks, so wrapping those callbacks with an event listener that waits for when the plugin is ready is a trivial task.

For the few [WebRTC] normalized methods that do not have a callback, like RTCPeerConnection() you need to take some brave decisions. Blocking IO and wasting CPU like you do today is not one of them, nor it is acceptable to do so on your clients' product.

You need to instruct developers that if they need WebRTC in IE, Safari (use the plugin in general) they have to wait for the "plugin ready" event before they do any WebRTC operations on the page, and you need to be clear and loud about it.

Until the plugin is ready, methods with no callback, like RTCPeerConnection() should throw an error prompting the developer to wait for the Plugin Ready event.

Developers, knowingly integrate with the Termasys plugin, meaning it's a choice that they use that specific adapter.js module and they setup their services / products to integrate accordingly. They wouldn't mind waiting for the event.

[serrynaimo]

@thanpolas We had an internal debate about this topic again today and we're going to bring the callback function back that you as a developer can override to get notified once the plugin is loaded. We'll also strongly recommend people that want to work with our plugin to use this callback.

We will still need the code loop in the RTCPeerConnection constructor to maintain compatibility with the existing adapter.js by Google and with our enterprise clients. This AdapterJS project was created to create a common API in all browsers (that includes our plugin), so that developers don't have to think about which environment they are working with as much as we can. We want to fulfill this promise, but will now offer educated developers a way to optimize their code using a plugin-ready callback.

Thanks for bringing it up again. Hope it helps you.

[johache]

Was implemented and release as part of AJS 0.10.4. https://github.com/Temasys/AdapterJS/releases

Closing the ticket