Skip to content

Commit

Permalink
[IMP] model: store handler arrays
Browse files Browse the repository at this point in the history
Currently the model create a new array every time `this.handlers` or
`this.coreHandlers` are accessed. That means that
we create new arrays every time we dispatch a command, leading to
quite a bit of garbage collection.

When deleting the first column of a 26 x 1000 sheet filled with numbers,
initializing the arrays saves ~100ms in garbage collection.

Task: 3272878
Part-of: #2340
  • Loading branch information
hokolomopo committed Apr 19, 2023
1 parent e0395f7 commit 997ccc6
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
19 changes: 13 additions & 6 deletions src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ export class Model extends EventBus<any> implements CommandDispatcher {

uuidGenerator: UuidGenerator;

private readonly handlers: CommandHandler<Command>[] = [];
private readonly coreHandlers: CommandHandler<CoreCommand>[] = [];

constructor(
data: any = {},
config: Partial<ModelConfig> = {},
Expand Down Expand Up @@ -194,6 +197,9 @@ export class Model extends EventBus<any> implements CommandDispatcher {
// Initiate stream processor
this.selection = new SelectionStreamProcessor(this.getters);

this.coreHandlers.push(this.range);
this.handlers.push(this.range);

// registering plugins
for (let Plugin of corePluginRegistry.getAll()) {
this.setupCorePlugin(Plugin, workbookData);
Expand All @@ -204,6 +210,8 @@ export class Model extends EventBus<any> implements CommandDispatcher {
}
this.uuidGenerator.setIsFastStrategy(false);

this.handlers.push(this.history);

// starting plugins
this.dispatch("START");
// Model should be the last permanent subscriber in the list since he should render
Expand All @@ -227,10 +235,6 @@ export class Model extends EventBus<any> implements CommandDispatcher {
markRaw(this);
}

get handlers(): CommandHandler<Command>[] {
return [this.range, ...this.corePlugins, ...this.uiPlugins, this.history];
}

joinSession() {
this.session.join(this.config.client);
}
Expand All @@ -251,6 +255,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
this.getters[name] = plugin[name].bind(plugin);
}
this.uiPlugins.push(plugin);
this.handlers.push(plugin);
const layers = Plugin.layers.map((l) => [plugin, l] as [UIPlugin, LAYERS]);
this.renderers.push(...layers);
this.renderers.sort((p1, p2) => p1[1] - p2[1]);
Expand Down Expand Up @@ -282,6 +287,8 @@ export class Model extends EventBus<any> implements CommandDispatcher {
}
plugin.import(data);
this.corePlugins.push(plugin);
this.coreHandlers.push(plugin);
this.handlers.push(plugin);
}

private onRemoteRevisionReceived({ commands }: { commands: CoreCommand[] }) {
Expand All @@ -305,7 +312,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
return;
}
this.isReplayingCommand = true;
this.dispatchToHandlers([this.range, ...this.corePlugins], command);
this.dispatchToHandlers(this.coreHandlers, command);
this.isReplayingCommand = false;
}
),
Expand Down Expand Up @@ -437,7 +444,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
const command: Command = { type, ...payload };
const previousStatus = this.status;
this.status = Status.RunningCore;
const handlers = this.isReplayingCommand ? [this.range, ...this.corePlugins] : this.handlers;
const handlers = this.isReplayingCommand ? this.coreHandlers : this.handlers;
this.dispatchToHandlers(handlers, command);
this.status = previousStatus;
return DispatchResult.Success;
Expand Down
2 changes: 1 addition & 1 deletion tests/xlsx_export.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { exportPrettifiedXlsx, toRangesData } from "./test_helpers/helpers";
function getExportedExcelData(model: Model): ExcelWorkbookData {
model.dispatch("EVALUATE_CELLS");
let data = createEmptyExcelWorkbookData();
for (let handler of model.handlers) {
for (let handler of model["handlers"]) {
if (handler instanceof BasePlugin) {
handler.exportForExcel(data);
}
Expand Down

0 comments on commit 997ccc6

Please sign in to comment.