Prevent $digest cycle to trigger on UpdateTime

[augnustin]

I have quite some performance issues with videogular, because each time the player triggers the UpdateTime event, a $digest cycle is triggered, which in my case result useless calculation.

Is there a way to control if the UpdateTime event should trigger a $digest cycle?

If not, would it make sense ?

[2fdevs]

You can override the API.onUpdateTime method:
https://github.com/2fdevs/videogular/blob/master/app/scripts/com/2fdevs/videogular/controllers/vg-controller.js#L125

Probably you will have some problems, specially with views that use time values (vgTimeDisplay, vgScrubBarCurrentTime, etc...).

[alexthewilde]

I've run into the same problem. Especially in conjunction with the updateTime interval in videogular-youtube, which triggers onUpdateTime and thus $scope.$apply every 600ms, this can essentially kill an app's performance.

I understand the side effects though of removing it completely as explained by @2fdevs. On the other hand, this is really a major issue and I think the design needs to be re-thought to overcome the side effects of removing $scope.$apply once and for all.

I've resorted to maintain a patched version of videogular for the time being with the digest loop call removed.

[Elecash]

@alexthewilde I understand that this is an issue but it is hard to solve in Angular 1, and the only thing that it comes to my mind needs a major refactor to make $digest instead of $apply.

Now I'm focused on v2 with ng2 which it should be the future and I'm not willing to make any major refactor now or breaking change to v1. My recommendation as I said is to patch videogular in a custom fork or override API.onUpdateTime with your custom implementation.

Thanks for understanding.

[alexthewilde]

@Elecash makes total sense. I'll maintain a custom fork of v1 as you suggested since performance is a dealbreaker for me here.

[augnustin]

@alexthewilde Does you code mean that if I write:

      $scope.nothingNeedsToChange = //...
      $scope.onUpdateTime = function(currentTime) {
        if($scope.nothingNeedsToChange) {
          if ($scope.$root.$$phase != '$apply' && $scope.$root.$$phase != '$digest') {
              $scope.$apply();
          }
        }
      });

with: <videogular vg-update-time="onUpdateTime($currentTime)"></videogular>, it will prevent a $digest cycle to occur when $scope.nothingNeedsToChange is true or it is more complicated than that?

Thanks

[goldcaddy77]

@aug-riedinger Did you figure out how to do this? I'm having a similar problem in one of my apps.

[augnustin]

After investigating in the videogular source code and regarding the fact that the angular v1 version is no longer maintained, I think I'll import this controller's code into my project and handle it manually (there are a lot of event listeners I don't need etc.).

If I achieve doing this + I feel a lot of performance improvement, I'll report my code here.

[goldcaddy77]

Cool thanks 👍

[alexthewilde]

hi @aug-riedinger, my solution was to replace $scope.$apply() which triggers a global digest cycle every n milliseconds and thus slows down an app significantly with $scope.$parent.$digest(). This limits the digest cycle scope to the videogular directive. You should see good performance improvements with this.

[Elecash]

@alexthewilde that is indeed an easy and fast fix, probably I'll add to close this thing forever. Thanks for the suggestion!

[augnustin]

That sounds like an interesting solution indeed.
I'd rather having this code available in my codebase instead of having to update the whole library. Is this possible?

If it were to change the <videogular> API, I'd do something like: if onVgUpdateTime or onVgUpdateState return true, then generate a $digest cycle, otherwise, do nothing (or maybe if three cases are needed, 0=nothing, 1=parent.$digest, 2+=$rootScope.$digest. But in my case I never need anything more than a parent.$digest);

If I look at my onVgUpdateTime method, I have the different cases (I changed the returns depending on my theoretical need):

      // Warning:
      // This method is called every second and triggers a digest cycle, so keep it light and simple
      $scope.onVgUpdateTime = function(currentTime) {
        // The player is not playing
        if ($scope.player.currentState !== 'play') { return 0; }

        // The player is "lost"
        if (!$scope.currentObject) {
          $scope.goTo($scope.currentObject());
          return 1;
        }

        // Still playing the same currentObject;
        if (_.inRange(currentTime, $scope.currentObject.starts_at, $scope.currentObject.ends_at)) {
          return 0;
        }

        if ($scope.watchingBalls()) {
          // Go to next ball
          $scope.goToNext();
          return 1;
        } else {
          if (_.inRange(currentTime, $scope.currentPoint.starts_at, $scope.currentPoint.ends_at)) {
            // Point is still valid, update ball
            $scope.currentObject = $scope.findCurrent($scope.currentPoint.balls, currentTime);
            return 0;
          }
          // Go to next point
          $scope.goToNext();
          return 1;
        }
      };

[Elecash]

I'm working right now in the simple fix provided by @alexthewilde but if you want to send after the new release an improvement I'm glad to review your PR 😄

[Elecash]

Fixed with 74faeb0

[alexthewilde]

@Elecash sweet! Are you sure though that using $scope.$parent.$digest() in all the other callbacks as well doesn't break functionality? I think I tested this once and had issues, that's why I reverted to only putting it into onUpdateTime.

[alexthewilde]

...which is not to say that the goal should be to make it work with $scope.$parent.$digest() of course.

[Elecash]

I've tested a little bit and everything seemed to work. In fact, it should work in that way, if there's a bug or other issues should not be related with $parend.$digest but with other problems that are hidden.