pubnub / pubnub-angular

Official PubNub AngularJS SDK
Other
84 stars 43 forks source link

ngUnsubscribe takes too long #7

Open PatrickJS opened 9 years ago

PatrickJS commented 9 years ago

ngUnsubscribe has poor performance compared to window.PUBNUB.unsubscribe() screen shot 2014-10-30 at 12 41 49 pm

stephenlb commented 9 years ago

Hi Patrick! some q's - Is this network time related? window.PUBNUB.unsubscribe() will default to a different key space separate from your account. perhaps the unsubscribe is just no-op for window.PUBNUB.unsubscribe().

PatrickJS commented 9 years ago

so this is instant

      c.ngUnsubscribe = function(args) {
        console.time('ngUnsubscribe');
        // var cpos;
        // cpos = c['_channels'].indexOf(args.channel);
        // if (cpos !== -1) {
        //   c['_channels'].splice(cpos, 1);
        // }
        // c['_presence'][args.channel] = null;
        // delete $rootScope.$$listeners[c.ngMsgEv(args.channel)];
        // delete $rootScope.$$listeners[c.ngPrsEv(args.channel)];
        return window.PUBNUB.unsubscribe(args, function() {
          console.timeEnd('ngUnsubscribe');
        });
      };

while the original code isn't https://github.com/pubnub/pubnub-angular/blob/master/lib/pubnub-angular.js#L144

      c.ngUnsubscribe = function(args) {
        console.time('ngUnsubscribe');
        // var cpos;
        // cpos = c['_channels'].indexOf(args.channel);
        // if (cpos !== -1) {
        //   c['_channels'].splice(cpos, 1);
        // }
        // c['_presence'][args.channel] = null;
        // delete $rootScope.$$listeners[c.ngMsgEv(args.channel)];
        // delete $rootScope.$$listeners[c.ngPrsEv(args.channel)];
        return c.jsapi.unsubscribe(args, function() {
          console.timeEnd('ngUnsubscribe');
        });
      };

the only thing I can think of is whatever this is trying to do https://github.com/pubnub/pubnub-angular/blob/master/lib/pubnub-angular.js#L15

      _ref = ['map', 'each'];
      for (_i = 0, _len = _ref.length; _i < _len; _i++) {
        k = _ref[_i];
        if ((typeof PUBNUB !== "undefined" && PUBNUB !== null ? PUBNUB[k] : void 0) instanceof Function) {
          (function(kk) {
            return c[kk] = function() {
              var _ref1;
              return (_ref1 = c['_instance']) != null ? _ref1[kk].apply(c['_instance'], arguments) : void 0;
            };
          })(k);
        }
      }
      for (k in PUBNUB) {
        if ((typeof PUBNUB !== "undefined" && PUBNUB !== null ? PUBNUB[k] : void 0) instanceof Function) {
          (function(kk) {
            return c['jsapi'][kk] = function() {
              var _ref1;
              return (_ref1 = c['_instance']) != null ? _ref1[kk].apply(c['_instance'], arguments) : void 0;
            };
          })(k);
        }
      }
MehulATL commented 9 years ago

:+1:

sunnygleason commented 9 years ago

@gdi2290 if you're using angular, you shouldn't be referencing PUBNUB - the Angular library exposes all direct methods via PubNub.jsapi., where \ is the regular JS methods. I have a feeling that your method is fast because the window.PUBNUB is not really connected, as Stephen alludes to...

I just created a codepen where you should be able to see what's going on:

http://codepen.io/sunnygleason/pen/FIiDs?editors=100

My performance looks like this...

subscribed in 95 ms
unsubscribed in 191 ms
subscribed in 66 ms
unsubscribed in 44 ms
ngsubscribed in 56 ms
ngunsubscribed in 32 ms
ngsubscribed in 51 ms
ngunsubscribed in 356 ms
ngsubscribed in 78 ms
ngunsubscribed in 34 ms

What do you see?

PatrickJS commented 9 years ago

I updated the codepen http://codepen.io/anon/pen/xFDtg?editors=100

for whatever reason it has to do with these lines

~~c.ngSubscribe = function(args) { var _base, _name; // start wat if (c['_channels'].indexOf(args.channel) < 0) { c['_channels'].push(args.channel); } // end wat (_base = c['_presence'])[_name = args.channel] || (_base[_name] = []); args = c._ngInstallHandlers(args); return c.jsapi.subscribe(args); }; c.ngUnsubscribe = function(args) { var cpos; // start wat cpos = c['_channels'].indexOf(args.channel); if (cpos !== -1) { c['_channels'].splice(cpos, 1); } // end wat c['_presence'][args.channel] = null; delete $rootScope.$$listeners[c.ngMsgEv(args.channel)]; delete $rootScope.$$listeners[c.ngPrsEv(args.channel)]; return c.jsapi.unsubscribe(args); };~~

if I comment out these lines I no longer have any lag in my app when signing out (unsubscribing)

~~c.ngSubscribe = function(args) { var _base, _name; // if (c['_channels'].indexOf(args.channel) < 0) { // c['_channels'].push(args.channel); // } (_base = c['_presence'])[_name = args.channel] || (_base[_name] = []); args = c._ngInstallHandlers(args); return c.jsapi.subscribe(args); }; c.ngUnsubscribe = function(args) { // var cpos; // cpos = c['_channels'].indexOf(args.channel); // if (cpos !== -1) { // c['_channels'].splice(cpos, 1); // } c['_presence'][args.channel] = null; delete $rootScope.$$listeners[c.ngMsgEv(args.channel)]; delete $rootScope.$$listeners[c.ngPrsEv(args.channel)]; return c.jsapi.unsubscribe(args); };~~

I'm unable to replicate the problem with codepen

the problem is most likely network time related as Stephen suggested. I can investigate this a little more later since hanging only in my app

PatrickJS commented 9 years ago

I found the problem, but the issue should be submitted on the PubNub javascript repo. The problems turns out to be synchronous xhr calls for presence events (Leave). Initializing pubnub with noleave: true works because it's not having these calls but that shouldn't be a solution. The synchronous calls are due to SSL in this bit on code

xdr({
  blocking : blocking || SSL,

https://github.com/pubnub/javascript/blob/88a10ec782f7298ecf5833ed3c9b9a7b1ab0298d/core/pubnub-common.js#L515

In the example I forgot to add ssl: true which is why I wasn't able to replicate my problem. Here is the updated example http://codepen.io/gdi2290/pen/ZYbjbR
and the pull request. https://github.com/pubnub/javascript/pull/50