store.upsert(items) is very slow ($rootScope.$apply)

[dnozay]

ok found that $rootScope.$apply is being called.

      notify: (args...) ->
        $rootScope.$apply =>
          @q.notify(args...)

and let's take a quick look at upsert:

      upsert: (data) ->
        @_arrayOperation data, (item) =>
          @store.put(item)

turns out that all array operations are using notify.

      _arrayOperation: (data, mapFunc) ->
        defer = @defer()
        data = [data] unless angular.isArray(data)
        for item in data
          req = mapFunc(item)
          results = []
          defer.rejectWith(req)
          req.onsuccess = (e) ->
            results.push(e.target.result)
            defer.notify(e.target.result)
            defer.resolve(results) if results.length >= data.length
        if data.length == 0
          return $q.when([])
        defer.promise

[dnozay]

            defer.notify(e.target.result)

this piece is making my UI very very slow. I am retrieving 1000 items from a REST call, processing them and inserting them in $indexedDB, but it starts 1000 digest cycles...

It would be great if we had a way to opt out from the notify calls.

[bramski]

I'm not sure if anybody cares too much about this. I don't see a big deal with adding configuration to opt-out and then making a major version upgrade so people don't get surprised. Sucks that angular $q has a perf issue here when nobody is listening for the updates and still issues the $digest.

[bramski]

Suggest you open a PR with a fix that will work for you and we can get it released fairly quickly. I don't have time to do this personally.

[dnozay]

It may be a different problem altogether. http://stackoverflow.com/questions/18499909/how-to-count-total-number-of-watches-on-a-page - there are 50k+ watches on the page; so I am not sure anymore if the problem is caused by notify or by the amount of watches on the page, making each notify expensive.

[bramski]

Can you close your issue? Is this a problem with this library or with your implementation?

[thorn0]

@bramski Throughout the library, you have a lot of $rootScope.$apply calls. Why are they needed if $q-promises trigger the digest cycle anyway?

[bramski]

Previous versions of angular did not do this. It may be able to be removed at this point. I'm unsure. Is someone who is seeing a perf issue going to benchmark and see?

[thorn0]

Previous versions of angular did not do this.

They always did AFAIK. That's one of the main reasons for Angular to have its own implementation of promises. See angular/angular.js#6697

[bramski]

Sure. The $rootScope.$apply was leftover from the original implementer. I had no reason to change it. If someone can show (a) that it's unnecessary (b) that it's detrimental to performance then sure let's remove it. I have no quantitative information right now.

[thorn0]

Don't have this information as well, but:

1. The unit tests fail (timeout) after removing $apply
2. These $apply calls sometimes cause the '$digest already in progress' exception. Don't know how to reproduce this. Mostly happens when I'm using debugger to debug some other part of the app.

[thorn0]

Researched it a bit more. My apologies, $q promises don't trigger the digest. Quite the contrary, they indeed need some code to trigger the digest for them, otherwise their then callbacks won't fire. In this connexion, it's interesting to look at the implementation of $http. First, they use if (!$rootScope.$$phase) $rootScope.$apply(); instead of just $rootScope.$apply(); to avoid the exception I mentioned above. Second, there is an interesting option that enables using $applyAsync instead of $apply.

[bramski]

Yeah. I haven't looked into this a lot. My recollection when I rewrote it
of the angular documentation here is kind of spotty.
On Jun 11, 2015 4:22 PM, "thorn0" notifications@github.com wrote:

Researched it a bit more. My apologies, $q promises don't trigger the
digest. Quite the contrary, they indeed need some code to trigger the
digest for them, otherwise their then callbacks won't fire. In this
connexion, it's interesting to look
https://github.com/angular/angular.js/blob/master/src/ng/http.js at the
implementation of $http. First, they use if (!$rootScope.$$phase)
$rootScope.$apply(); instead of just $rootScope.$apply(); to avoid the
exception I mentioned above. Second, there is an interesting option that
enables using $applyAsync
https://docs.angularjs.org/api/ng/type/%24rootScope.Scope#%24applyAsync
instead of $apply.

—
Reply to this email directly or view it on GitHub
#19 (comment)
.

[theBull]

$rootScope.$apply is used because the original author created his own implementation of a deferred/promise model called DbQ. It acts as a wrapper around Angular's $q service. I was curious about this in Issue # 39: Is the DbQ object really necessary? Why not just use $q

I tinkered with this a little by replacing the DbQ deferred promise in a couple places with the $q.defer() object, but I believe we'd have to revamp the entire architecture of deferred promises within the library in order to effectively handle this. In any case, that would enable us to get rid of the $rootScope.$apply calls.

As far as doing a bulk notify call after a series of upserts, I am totally onboard with that. I have exactly the same scenario - loading 1,000 items over a REST request. Because these upserts are individually bound to the $apply call, and because Angular renders bindings in the UI when a $digest occurs (is that correct?), the high number of digests are causing a rendering freeze.

[thorn0]

$rootScope.$apply is used because the original author created his own implementation of a deferred/promise model called DbQ

No. It's needed for Angular promises. Their callbacks are called on digest. See my previous comment. It seems to me, using $applyAsync can help in your case.