15

We have an Angular 2 site with a websocket pumping in data from backend to our grid. To indicate the recent updates we use CSS to set background color for the row and bold font on cells that are affected.

The indication should only last a short time.

1) Our first attempt was to reset all indicators when the next batch arrived from server. This was working well but in some views there are very seldom updates which means the indicators can stay very long which is kind of confusing.

It would be more consistent if update indicators disappeared after a fixed interval, like 4 seconds.

2) Our next attempt was to use CSS animations. But after a while it was lagging a lot. The impression is that too many animations running will overload the browser not coping with the requested times. Maybe each animation has its own timer in the background?

3) The third attempt is to have one timer running at fixed intervals and then checking what records to reset. We have created a TimerService that will regularly check for due items. When adding an item to the timer pool it can be configured with arbitrary wait time.

This works, but in the log window there are frequent violation warnings:

[Violation] 'setInterval' handler took 56ms
[Violation] 'setInterval' handler took 74ms
[Violation] 'setInterval' handler took 63ms
[Violation] 'setInterval' handler took 88ms
...

But when we time what happens inside the checkItems method it just takes 0.03ms!

  • We all have C# background and have just been working with Angular for some months. Maybe we are imposing a backend approach?

  • Is there a context switching going on that we have missed?

  • Is there an alternative more frontend friendly approach?

  • Are there some crucial optimizations we could do to our code?

All suggestions are appreciated!

Here is the suggested TimerService causing all warnings:

import { Injectable, OnInit } from "@angular/core";
import { Observable } from "rxjs/Rx";
import { Subject } from "rxjs/Subject";

@Injectable()
export class TimerService {
    private timerItems: TimerItem[] = [];
    private dueTimeReachedSubject: Subject<string> = new Subject<string>();
    public dueTimeReached: Observable<string> = this.dueTimeReachedSubject.asObservable();

    constructor() {
        setInterval(() => this.checkItems(), 1000);
    }

    private checkItems() {
        let now = Date.now();
        let removeKeys: string[] = [];
        this.timerItems.filter(t => t.dueTime <= now).forEach(t => {
            this.dueTimeReachedSubject.next(t.key);
            removeKeys.push(t.key);
        });
        this.timerItems = this.timerItems.filter(t => removeKeys.indexOf(t.key) < 0);
    }

    public add(key: string, delayInSeconds: number) {
        let dueTime = Date.now() + delayInSeconds * 1000;
        let timerItem = this.timerItems.find(t => t.key === key);
        if (timerItem) {
            timerItem.dueTime = dueTime;
        }
        else {
            this.timerItems.push(new TimerItem(key, dueTime));
        }
    }   

    public remove(key: string) {
        this.timerItems = this.timerItems.filter(t => t.key !== key);
    }
}


class TimerItem {
    constructor(public key: string, public dueTime: number) { }
}

EDIT

I tried to use Observable.interval: Same result with exactly the same warning message: "[Violation] 'setInterval' handler took xx ms"

I tried to use setTimeout with repeated calls: Same result but with modified warning message: "[Violation] 'setTimeout' handler took xx ms"

I even tried to empty the checkItems of every line and I still get the warnings.

The warning is thrown from within zone.js and seems to be an Angular inner recommendation. I know I can turn off verbose logging in Chrome developer tools, but I often use console.debug for development purposes and that means they will disappear as well.

A warning is OK I guess for slow functions, but in this case it is just triggering a setInterval function. Why would that be slow?

Jakob Lithner
  • 3,923
  • 6
  • 32
  • 55
  • pretty blind suggestions : what happens if you time : `setInterval( () => { start; this.checkItems(); end; }, 1000);` ? is `0.03 ms` a consistent measure, which you also observe when Chrome logs `'setInterval' handler took 74ms` ? – LeGEC Oct 16 '18 at 07:38
  • can you test your class without the `@Injectable` decorator ? – LeGEC Oct 16 '18 at 08:00
  • This is not a general solution, but you can apply a class to a batch of rows and make it fade out all together. It would save a lot of calculations – Cristian Traìna Oct 16 '18 at 08:23
  • In this context, you should also use `ChangeDetectionStrategy.OnPush` if the process is handled by a smart component with many children – Cristian Traìna Oct 16 '18 at 08:26
  • `Array.filter` is causing this, try basic `for` loop and see if it's persist – line88 Oct 22 '18 at 18:23
  • here is the very basic jsbench test - http://jsben.ch/7Zv4N and here is the good answer why - https://stackoverflow.com/a/31471843/1816407 – line88 Oct 22 '18 at 18:38

5 Answers5

3

there are numerous things that can be done in a stations like this.

the first one is to change the ChangeDetectionStrategy to ChangeDetectionStrategy.onPush , then active the detection only when needed, in your case at the end of checkItems ,add and remove . this will reduce dramatically the scripting as angular will need to evaluate your html only when asked

the second thing you can do is to check if your temple have a function call in an *ngIf or an *ngFor, if you do angular will not be able to cache the values this functions returns and will have to process them for each check

the third thing you should consider is optimize checkItems, checkItems runtime in O(n^2) and does many unnecessary loops. you can reduce it

checkItems() {
  this.timerItems = this.timerItems.filter(t => {
      if(t.dueTime <= now){
          this.dueTimeReachedSubject.next(t.key);
          return false;
      }
      return true;
  });        
}

for small arrays it wont help a lot but the bigger the get this change will start take effect..

i guss more things can be done but this 3 things help me in my similar performance issues

Amit Wagner
  • 2,370
  • 3
  • 9
  • 28
2

I would ditch the setInterval and use a setTimeout only when necessary.

import { Injectable, OnInit } from "@angular/core";
import { Observable } from "rxjs/Rx";
import { Subject } from "rxjs/Subject";

let timer = null

@Injectable()
export class TimerService {
    private timerItems: TimerItem[] = [];
    private dueTimeReachedSubject: Subject<string> = new Subject<string>();
    public dueTimeReached: Observable<string> = this.dueTimeReachedSubject.asObservable();

    constructor() {
        this.checkItems();
    }

    private checkItems() {
        // clear the timeout
        clearTimeout(timer)
        let now = Date.now();
        let removeKeys: string[] = [];
        this.timerItems.filter(t => t.dueTime <= now).forEach(t => {
            this.dueTimeReachedSubject.next(t.key);
            removeKeys.push(t.key);
        });
        this.timerItems = this.timerItems.filter(t => removeKeys.indexOf(t.key) < 0);
        // only use the timer if there are timerItems
        if(this.timerItems.length) {
            timer = setTimeout(() => this.checkItems(), 1000)
        }
    }

    public add(key: string, delayInSeconds: number) {
        let dueTime = Date.now() + delayInSeconds * 1000;
        let timerItem = this.timerItems.find(t => t.key === key);
        if(timerItem) {
            timerItem.dueTime = dueTime;
        }
        else {
            this.timerItems.push(new TimerItem(key, dueTime));
        }
        // check for items and engage the timer if necessary
        this.checkItems()
    }   

    public remove(key: string) {
        this.timerItems = this.timerItems.filter(t => t.key !== key);
    }
}


class TimerItem {
    constructor(public key: string, public dueTime: number) { }
}
Ty Kroll
  • 1,329
  • 11
  • 24
1

As I understand you want css changes to disappear after a certain interval.

  1. Have css classes for normal and updates states of cells and rows. e.g.

.row-normal{
//css-styling goes here
}
.row-updates{
//css- here
}

---approach 1--- set on update listener define onChange attribute to your views in html or by using document.getElementByTagName in JavaScript Code.

function onUpdate(){
  let v = this;
  v.className = "row-updated";
  setTimeOut(function(){v.className = "row-normal"}, interval_in_millis)
}

Hope this helps.

mayank1513
  • 4,971
  • 3
  • 23
  • 70
0

Are you sure that you re-built your project after clearing the code from checkItems.

As far as I know, it is complaining that the checkItems function is taking too long.

Also just in case, I did setup a StackBlitz here

With your code and was not able to recreate it..

Maybe you can fork the StackBlitz and try to recreate the issue there if you're still having problems?

JBoothUA
  • 2,465
  • 3
  • 15
  • 40
0

One solution would be to use Angular's animations.

For example, elements that are added to a list are a case of :enter transition (which is an alias to void => * transition). From the linked documentation:

HTML:

<div @myInsertRemoveTrigger *ngIf="isShown" class="insert-remove-container">
  <p>The box is inserted</p>
</div>

TS:

trigger('myInsertRemoveTrigger', [
  transition(':enter', [
    style({ opacity: 0 }),
    animate('5s', style({ opacity: 1 })),
  ]),
  transition(':leave', [
    animate('5s', style({ opacity: 0 }))
  ])
]),

When element with myInsertRemoveTrigger trigger would "come out from void" (:enter) it would initially get style opacity: 0 and then transition to opacity: 1 for 5 seconds.

The animate()'s first parameter ('5s' in example) can be also used to define delay and easing. You mentioned the "new item" style could be applied for 4 seconds, so the transition rule could be for example:

  transition(':enter', [
    style({ backgroundColor: 'green' }),
    animate('1s 4s ease-in', style({ backgroundColor: 'white' })),
  ])

This would apply green background on new items immediately and then, after 4 seconds, it would give them white background color (with transition length of 1 second).

Jari Keinänen
  • 1,259
  • 1
  • 21
  • 41