Skip to content

Commit

Permalink
perf(virtual-scroll): use auditTime instead of sampleTime (#13131)
Browse files Browse the repository at this point in the history
The `CdkVirtualScrollViewport` uses `sampleTime` to collect multiple scroll events into one until the next animation frame. 
However the `sampleTime` also samples even if there is no new upstream values. Which in this case creates an basically useless requestAnimationFrame callback.
A better choice would be the `auditTime` operator, which only schedules the requestAnimationFrame when there is actually a new upstream value.
In our case it has the exact same behavior, as from looking at the performance profiler in chrome I found out this: first the scroll event fires, then pending requestAnimationFrame callbacks get called, regardless if they where scheduled in the current or previous frame and then the paint occurs.
  • Loading branch information
nename0 authored and mmalerba committed Sep 21, 2018
1 parent bfeb540 commit c26dc74
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions src/cdk/scrolling/virtual-scroll-viewport.ts
Expand Up @@ -24,7 +24,7 @@ import {
ViewEncapsulation,
} from '@angular/core';
import {animationFrameScheduler, Observable, Subject} from 'rxjs';
import {sampleTime, startWith, takeUntil} from 'rxjs/operators';
import {auditTime, startWith, takeUntil} from 'rxjs/operators';
import {ScrollDispatcher} from './scroll-dispatcher';
import {CdkScrollable, ExtendedScrollToOptions} from './scrollable';
import {CdkVirtualForOf} from './virtual-for-of';
Expand Down Expand Up @@ -144,9 +144,10 @@ export class CdkVirtualScrollViewport extends CdkScrollable implements OnInit, O
.pipe(
// Start off with a fake scroll event so we properly detect our initial position.
startWith<Event | null>(null!),
// Sample the scroll stream at every animation frame. This way if there are multiple
// scroll events in the same frame we only need to recheck our layout once.
sampleTime(0, animationFrameScheduler))
// Collect multiple events into one until the next animation frame. This way if
// there are multiple scroll events in the same frame we only need to recheck
// our layout once.
auditTime(0, animationFrameScheduler))
.subscribe(() => this._scrollStrategy.onContentScrolled());

this._markChangeDetectionNeeded();
Expand Down

0 comments on commit c26dc74

Please sign in to comment.