akoenig / angular-deckgrid

A lightweight masonry-like grid for AngularJS.
http://akoenig.github.io/angular-deckgrid
MIT License
1.1k stars 191 forks source link

CPU usage constant not expected #10

Open marcalj opened 10 years ago

marcalj commented 10 years ago

Hi, if I have the website open, also in background, it's always eating 8% CPU (MacBookPro 2013 - dual core).

Please take a look in performance in $digests, loops, whatever. I'm expecting nearly 0% cpu processing there. There's some tasks always running in background?

Thanks!

akoenig commented 10 years ago

Hey,

I think there is some optimization potential in some digests. The culprits might be the model watcher or the newly created scopes within the respective card loops. I guess there is the problem. The directive itself does not run any background jobs.

Thanks for the heads up. I added it to the roadmap and I will take a closer look as soon as I can.

/André

marcalj commented 10 years ago

Great! :+1:

mikenikles commented 10 years ago

I have a similar situation, especially as the source array grows. In my case, an infinite-scroll directive keeps on adding items to the source array. Each time that happens, the $$onModelChange watcher is being fired, which in turn calls $$createColumns(). I suspect the CPU issue to be in that said $$createColumns() function when the angular.forEach loop is executed and the columns being re-created.

Ideally, I think, there should be a way to extend the this.$$scope.columns array vs. rebuilding it every time the source array changes.

I'll look into it as well and see if I can come up with something.

mikenikles commented 10 years ago

Caution: This is a temporary solution, as it doesn't work when elements are removed from the source, which is ok in my case for now. I'll try to spend some more time fixing this as well.

I've rewritten the $$createColumns function as per below.

Deckgrid.prototype.$$createColumns = function $$createColumns () {
    var self = this;

    if (!this.$$scope.layout) {
        return $log.error('angular-deckgrid: No CSS configuration found (see ' +
            'https://github.com/akoenig/angular-deckgrid#the-grid-configuration)');
    }

    angular.forEach(this.$$scope.model, function onIteration (card, index) {
        var column = (index % self.$$scope.layout.columns) | 0;

        if (!self.$$scope.columns[column]) {
            self.$$scope.columns[column] = [];
        }

        var isCardAvailable = false;
        angular.forEach(self.$$scope.columns[column], function(existingCard, cardInColumnIndex) {
            isCardAvailable = isCardAvailable || angular.equals(existingCard, card);
        });

        if (!isCardAvailable) {
            self.$$scope.columns[column].push(card);
        }
    });
};```
jjjjeeffff commented 10 years ago

Had a similar issue... plus the screen was flickering when there were too many items (dynamically added), so appending vs. redrawing on every change was ideal. Came up with a similar solution in my fork (it doesn't account for elements being removed from the model very gracefully, but it does redraw everything in that case).

phuongnd08 commented 9 years ago

I take some idea from @mikenikles and try to speed up the render. I just opened a pull request. @akoenig Would be cool if you can have a look (and merge :) )

marcalj commented 9 years ago

@phuongnd08 Great!