gimite / web-socket-js

HTML5 Web Socket implementation powered by Flash
BSD 3-Clause "New" or "Revised" License
2.73k stars 489 forks source link

Lag #50

Closed mrjoes closed 13 years ago

mrjoes commented 13 years ago

Hello,

With flash socket implementation, I'm seeing significant lag for all incoming messages. Here are timings from simple ping application. Log format is: [client start mm:ss:ms] - [client end mm:ss:ms] - [client delta] - [server msg receive date] Native websockets in Chrome (10.0.628.0 dev): 32:11.952 - 32:11.954 - 2 - 32:11.952 32:32.464 - 32:32.466 - 2 - 32:32.466 32:46.116 - 32:46.117 - 1 - 32:46.117

Average: 1.5 ms.

Flash websocket, FF 3.6.12: 35:33.724 - 35:33.922 - 191 - 35:33.726 35:57.925 - 35:58.178 - 253 - 35:57.932 36:09.905 - 36:10.365 - 460 - 36:09.913

Flash websocket, IE 8: 37:05.725 - 37:06.185 - 460 - 37:05.727 37:20.787 - 37:21.211 - 424 - 37:20.789 37:34.220 - 37:34.736 - 516 - 37:34.225

Flash websocket, Opera 10: 39:59.038 - 39:59.165 - 127 - 39:59.047 40:11.825 - 40:12.165 - 340 - 40:11.834 40:35.937 - 40:36.165 - 228 - 40:35.946

Average is ~250 ms.

After some research I found that issue caused by following snippet of code: // This is to avoid "You are trying to call recursively into the Flash Player ..." // error which (I heard) happens when you pass bunch of messages. // This workaround was written here: // http://www.themorphicgroup.com/blog/2009/02/14/fabridge-error-you-are-trying-to-call-recursively-into-the-flash-player-which-is-not-allowed/ FABridge.EventsToCallLater["flash.events::Event"] = "true";

If you comment this line, I'm seeing ~9 ms response time in all browsers.

Question is:

  1. Is it really required?
  2. What is side effect of using it?
  3. Original link in the comment is no longer available. What is the cause of such errors?

Thanks.

gimite commented 13 years ago

Actually I'm still not exactly sure how the "call recursively" error was caused. Also I'm not sure what the FABridge.EventsToCallLater does. I believe the article said that the error may be fixed by it, so I just added it.

Recently I got a report that the error can still happen: https://github.com/gimite/web-socket-js/issues/closed/#issue/47 So I submitted a change to switch to setTimeout() hack instead of EventsToCallLater: https://github.com/gimite/web-socket-js/commit/20f837425d44144f0df7020a5f98350be2b83745

Can you try it? I don't know if the lag has got better or worse with the change, though.

mrjoes commented 13 years ago

FABridge.EventsToCallLater tells to do setTimeout for some of the propagated events instead of calling them directly. So, basically, code above tells FABridge to delay all "flash.events::Event" instances for 200+ ms.

I will check it, but looking through the changes I can surely say that it won't delay events using FABridge functionality. Dunno if it won't trigger the error as well.

kanaka commented 13 years ago

Your setTimeout invocation in handleEvents isn't quite right. It causes "Uncaught exception: TypeError: 'this. handleEvents' is not a function" every time it triggers.

This is because the internal function is referring to the new value of "this" (one of the worst behaviors of Javascript). Here is my suggested fix:

diff --git a/include/web-socket-js/web_socket.js b/include/web-socket-js/web_socket.js
index 2a7da35..48ec0fc 100755
--- a/include/web-socket-js/web_socket.js
+++ b/include/web-socket-js/web_socket.js
@@ -165,8 +165,9 @@
           if (this.__timer) clearInterval(this.__timer);
           if (window.opera) {
             // Workaround for weird behavior of Opera which sometimes drops events.
+            var that = this;
             this.__timer = setInterval(function () {
-              this.__handleEvents();
+              that.__handleEvents();
             }, 500);
           }
           if (this.onopen) this.onopen();
gimite commented 13 years ago

As kanaka reported in: https://github.com/gimite/web-socket-js/issues#issue/53 it seems the performance has been improved, say, except for Opera.