Skip to content

Commit

Permalink
Aggressively throttle all observers of all messages
Browse files Browse the repository at this point in the history
Summary: Since it's possible for the state to change very rapidly (multiple providers responding to keypress), we should throttle observers to this.

Reviewed By: paulcbetts

Differential Revision: D13006502

fbshipit-source-id: bce81b02ef1b3cc7ec3d8a1697bc63d3f8ad88cf
  • Loading branch information
Will Binns-Smith authored and pelmers committed Nov 15, 2018
1 parent 925fcf5 commit a5e3c57
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ describe('createStore', () => {
expect(spy_fileA.mock.calls[spy_fileA.mock.calls.length - 1]).toEqual([
{filePath: 'fileA', messages: [fileMessageA], totalMessages: 1},
]);
await sleep(600); // wait for the observeMessages throttle cooldown
expect(spy_allMessages.mock.calls.length).toBe(2);
expect(
spy_allMessages.mock.calls[spy_allMessages.mock.calls.length - 1][0],
Expand Down Expand Up @@ -163,6 +164,7 @@ describe('createStore', () => {
messages: [],
totalMessages: 0,
});
await sleep(600); // wait for the observeMessages throttle cooldown
expect(spy_allMessages.mock.calls.length).toBe(1);
expect(
spy_allMessages.mock.calls[spy_allMessages.mock.calls.length - 1][0],
Expand Down Expand Up @@ -229,6 +231,7 @@ describe('createStore', () => {
expect(spy_fileA.mock.calls[spy_fileA.mock.calls.length - 1]).toEqual([
{filePath: 'fileA', messages: [fileMessageA2], totalMessages: 1},
]);
await sleep(600); // wait for the observeMessages throttle cooldown
expect(spy_allMessages.mock.calls.length).toBe(2);
expect(
spy_allMessages.mock.calls[spy_allMessages.mock.calls.length - 1][0],
Expand Down Expand Up @@ -277,6 +280,7 @@ describe('createStore', () => {
expect(spy_fileA.mock.calls[spy_fileA.mock.calls.length - 1]).toEqual([
{filePath: 'fileA', messages: [], totalMessages: 0},
]);
await sleep(600); // wait for the observeMessages throttle cooldown
expect(spy_allMessages.mock.calls.length).toBe(2);
expect(
spy_allMessages.mock.calls[spy_allMessages.mock.calls.length - 1][0],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,19 @@ import {arrayEqual, mapEqual} from 'nuclide-commons/collection';
import UniversalDisposable from 'nuclide-commons/UniversalDisposable';
import {Observable} from 'rxjs';

const THROTTLE_FILE_MESSAGE_MS = 100;
// Receiving all messages is potentially dangerous as there can sometimes be
// tens of thousands, and updates can occur **on keystroke**. Throttle these to
// half of a second.
const THROTTLE_ALL_MESSAGES_MS = 500;
const THROTTLE_FILE_MESSAGES_MS = 100;

export default class DiagnosticUpdater {
_store: Store;
_states: Observable<AppState>;
_allMessageUpdates: Observable<Array<DiagnosticMessage>>;

constructor(store: Store) {
this._store = store;
this._states = observableFromReduxStore(store);

this._allMessageUpdates = this._states
.map(Selectors.getMessages)
.distinctUntilChanged();
}

getMessages = (): Array<DiagnosticMessage> => {
Expand All @@ -62,7 +61,13 @@ export default class DiagnosticUpdater {
observeMessages = (
callback: (messages: Array<DiagnosticMessage>) => mixed,
): IDisposable => {
return new UniversalDisposable(this._allMessageUpdates.subscribe(callback));
return new UniversalDisposable(
this._states
.let(throttle(THROTTLE_ALL_MESSAGES_MS))
.map(Selectors.getMessages)
.distinctUntilChanged()
.subscribe(callback),
);
};

observeFileMessagesIterator = (
Expand All @@ -72,7 +77,7 @@ export default class DiagnosticUpdater {
return new UniversalDisposable(
this._states
.distinctUntilChanged((a, b) => a.messages === b.messages)
.let(throttle(THROTTLE_FILE_MESSAGE_MS))
.let(throttle(THROTTLE_FILE_MESSAGES_MS))
.map(state => [
Selectors.getProviderToMessagesForFile(state)(filePath),
state,
Expand All @@ -99,7 +104,7 @@ export default class DiagnosticUpdater {
// Whether that's worth it depends on how often this is actually called with the same path.
this._states
.distinctUntilChanged((a, b) => a.messages === b.messages)
.let(throttle(THROTTLE_FILE_MESSAGE_MS))
.let(throttle(THROTTLE_FILE_MESSAGES_MS))
.map(state => Selectors.getFileMessageUpdates(state, filePath))
.distinctUntilChanged(
(a, b) =>
Expand Down

0 comments on commit a5e3c57

Please sign in to comment.