Skip to content

Commit

Permalink
[PERF] model: avoid useless finalize on initialization
Browse files Browse the repository at this point in the history
Currently, finalize will be called after each replay of a
`stateUpdateMessage` during the Model instanciation. Depending on the
type of revisions and on the number of cells to evaluate, this can end
up having a devastating effect on the performance at first load.

2 examples:
- on a spreadsheet on 450*26 cells (all filled) with 300 random but
  simple revisions, we spend 2.8 seconds in the evaluaion
- A client spreadsheet in 16.0 with 1100 revisions cannot load due to a
  crash of the broweer tab

This revision adds a "loading state" where the finalize is disabled and
only used once after the replay of all stateUpdateMessages.

closes #4969

Task: 4176216
X-original-commit: 512617b
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
rrahir committed Sep 13, 2024
1 parent 2d666d7 commit 81fe3c0
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
*/
private renderers: [UIPlugin, LAYERS][] = [];

private isLoading: boolean;
/**
* Internal status of the model. Important for command handling coordination
*/
Expand Down Expand Up @@ -219,6 +220,11 @@ export class Model extends EventBus<any> implements CommandDispatcher {
this.selection.observe(this, {
handleEvent: () => this.trigger("update"),
});

// move in "Loading" mode where we ignore redundant calls to finalize triggered by the
// replay of stateUpdateMessages in the session
this.isLoading = true;

// This should be done after construction of LocalHistory due to order of
// events
this.setupSessionEvents();
Expand All @@ -233,6 +239,10 @@ export class Model extends EventBus<any> implements CommandDispatcher {
// mark all models as "raw", so they will not be turned into reactive objects
// by owl, since we do not rely on reactivity
markRaw(this);

this.isLoading = false;
// ensure propre recomputation of plugin states after the message replays
this.finalize();
}

joinSession() {
Expand Down Expand Up @@ -368,6 +378,9 @@ export class Model extends EventBus<any> implements CommandDispatcher {
}

private finalize() {
if (this.isLoading) {
return;
}
this.status = Status.Finalizing;
for (const h of this.handlers) {
h.finalize();
Expand Down

0 comments on commit 81fe3c0

Please sign in to comment.