Skip to content

Commit

Permalink
perf(scroll-dispatcher): lazily subscribe to global events (#3270)
Browse files Browse the repository at this point in the history
* perf(scroll-dispatcher): lazily subscribe to global events

Switches the `ScrollDispatcher` to only subscribe to `scroll` and `resize` events if there any registered callbacks. Also unsubscribes once there are no more callbacks.

Fixes #3237.

* refactor: account for the `scrolled` subscriptions when adding the global listeners

* fix: wrong value being passed in

* fix: don't add global listener for scrollables
  • Loading branch information
crisbeto authored and tinayuangao committed Mar 9, 2017
1 parent d78a370 commit c1004cb
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/lib/core/overlay/position/viewport-ruler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export class ViewportRuler {
this._cacheViewportGeometry();

// Subscribe to scroll and resize events and update the document rectangle on changes.
scrollDispatcher.scrolled().subscribe(() => this._cacheViewportGeometry());
scrollDispatcher.scrolled(null, () => this._cacheViewportGeometry());
}

/** Gets a ClientRect for the viewport's bounds. */
Expand Down
25 changes: 24 additions & 1 deletion src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('Scroll Dispatcher', () => {
// Listen for notifications from scroll service with a throttle of 100ms
const throttleTime = 100;
let hasServiceScrollNotified = false;
scroll.scrolled(throttleTime).subscribe(() => { hasServiceScrollNotified = true; });
scroll.scrolled(throttleTime, () => { hasServiceScrollNotified = true; });

// Emit a scroll event from the scrolling element in our component.
// This event should be picked up by the scrollable directive and notify.
Expand Down Expand Up @@ -89,6 +89,29 @@ describe('Scroll Dispatcher', () => {
expect(scrollableElementIds).toEqual(['scrollable-1', 'scrollable-1a']);
});
});

describe('lazy subscription', () => {
let scroll: ScrollDispatcher;

beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => {
scroll = s;
}));

it('should lazily add global listeners as service subscriptions are added and removed', () => {
expect(scroll._globalSubscription).toBeNull('Expected no global listeners on init.');

const subscription = scroll.scrolled(0, () => {});

expect(scroll._globalSubscription).toBeTruthy(
'Expected global listeners after a subscription has been added.');

subscription.unsubscribe();

expect(scroll._globalSubscription).toBeNull(
'Expected global listeners to have been removed after the subscription has stopped.');
});

});
});


Expand Down
46 changes: 33 additions & 13 deletions src/lib/core/overlay/scroll/scroll-dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {Subject} from 'rxjs/Subject';
import {Observable} from 'rxjs/Observable';
import {Subscription} from 'rxjs/Subscription';
import 'rxjs/add/observable/fromEvent';
import 'rxjs/add/observable/merge';
import 'rxjs/add/operator/auditTime';


Expand All @@ -19,25 +20,26 @@ export class ScrollDispatcher {
/** Subject for notifying that a registered scrollable reference element has been scrolled. */
_scrolled: Subject<void> = new Subject<void>();

/** Keeps track of the global `scroll` and `resize` subscriptions. */
_globalSubscription: Subscription = null;

/** Keeps track of the amount of subscriptions to `scrolled`. Used for cleaning up afterwards. */
private _scrolledCount = 0;

/**
* Map of all the scrollable references that are registered with the service and their
* scroll event subscriptions.
*/
scrollableReferences: Map<Scrollable, Subscription> = new Map();

constructor() {
// By default, notify a scroll event when the document is scrolled or the window is resized.
Observable.fromEvent(window.document, 'scroll').subscribe(() => this._notify());
Observable.fromEvent(window, 'resize').subscribe(() => this._notify());
}

/**
* Registers a Scrollable with the service and listens for its scrolled events. When the
* scrollable is scrolled, the service emits the event in its scrolled observable.
* @param scrollable Scrollable instance to be registered.
*/
register(scrollable: Scrollable): void {
const scrollSubscription = scrollable.elementScrolled().subscribe(() => this._notify());

this.scrollableReferences.set(scrollable, scrollSubscription);
}

Expand All @@ -53,18 +55,36 @@ export class ScrollDispatcher {
}

/**
* Returns an observable that emits an event whenever any of the registered Scrollable
* Subscribes to an observable that emits an event whenever any of the registered Scrollable
* references (or window, document, or body) fire a scrolled event. Can provide a time in ms
* to override the default "throttle" time.
*/
scrolled(auditTimeInMs: number = DEFAULT_SCROLL_TIME): Observable<void> {
// In the case of a 0ms delay, return the observable without auditTime since it does add
// a perceptible delay in processing overhead.
if (auditTimeInMs == 0) {
return this._scrolled.asObservable();
scrolled(auditTimeInMs: number = DEFAULT_SCROLL_TIME, callback: () => any): Subscription {
// In the case of a 0ms delay, use an observable without auditTime
// since it does add a perceptible delay in processing overhead.
let observable = auditTimeInMs > 0 ?
this._scrolled.asObservable().auditTime(auditTimeInMs) :
this._scrolled.asObservable();

this._scrolledCount++;

if (!this._globalSubscription) {
this._globalSubscription = Observable.merge(
Observable.fromEvent(window.document, 'scroll'),
Observable.fromEvent(window, 'resize')
).subscribe(() => this._notify());
}

return this._scrolled.asObservable().auditTime(auditTimeInMs);
// Note that we need to do the subscribing from here, in order to be able to remove
// the global event listeners once there are no more subscriptions.
return observable.subscribe(callback).add(() => {
this._scrolledCount--;

if (this._globalSubscription && !this.scrollableReferences.size && !this._scrolledCount) {
this._globalSubscription.unsubscribe();
this._globalSubscription = null;
}
});
}

/** Returns all registered Scrollables that contain the provided element. */
Expand Down
2 changes: 1 addition & 1 deletion src/lib/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export class MdTooltip implements OnInit, OnDestroy {
ngOnInit() {
// When a scroll on the page occurs, update the position in case this tooltip needs
// to be repositioned.
this.scrollSubscription = this._scrollDispatcher.scrolled(SCROLL_THROTTLE_MS).subscribe(() => {
this.scrollSubscription = this._scrollDispatcher.scrolled(SCROLL_THROTTLE_MS, () => {
if (this._overlayRef) {
this._overlayRef.updatePosition();
}
Expand Down

0 comments on commit c1004cb

Please sign in to comment.