derbyjs / racer

Realtime model synchronization engine for Node.js
1.18k stars 116 forks source link

Improve performance when there are many reactive functions #264

Closed nateps closed 3 years ago

nateps commented 5 years ago

Implements EventTree, which uses a tree that mirrors the path structure. The interface is somewhat like an event listener, but the listeners may be any type of value, not just a function. Instead of emit(), there is a forEach() method, that is analogous to the Array.forEach() method but filtered to the listeners that may be impacted.

This PR also adapts model.start() to use EventTree instances instead of looping through a flat map of reactive functions. Could also be used to re-implement other model event listeners in racer that have a similar fan out emission pattern limited to specific paths.

The performance downside is that we're doing more work inside of model.start() and model.stop() for bookkeeping.

ericyhwang commented 5 years ago

Found a regression in testing.

It is possible to call model.start with the same output path more than once.

Previously, it would be last-one-wins, where the most recent Fn object getting set onto fromMap. The old listener would get overwritten and then garbage collected.

Now, the old listener doesn't get cleaned up from the event tree when calling model.start again with the same output path, so old supposedly-overwritten listeners stick around in memory in the tree. That's the case even when doing a model.unload(), since that calls stopAll, which loops over the fromMap.

Here's an example:

const outputPath = '_page.totalSize';
const inputPath = '_page.book';
const reactiveFn = (book) => { ... };
model.start(outputPath, inputPath, reactiveFn);
model.start(outputPath, inputPath, reactiveFn);  // Might be the same reactiveFn, might be a different one.
model.stop(outputPath);
// This should no longer have a node for `_page`, but it does.
console.log(model._fns.inputListeners.children);