Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ForceSimulation keeps redundantly re-initializing all forces any time any of their parameters change #201

Closed
regexident opened this issue Jun 23, 2024 · 1 comment · Fixed by #205 or #209

Comments

@regexident
Copy link
Contributor

regexident commented Jun 23, 2024

ForceSimulation (when uses as in the examples) keeps redundantly(!) re-initializing all(!) forces any time any of their parameters change.

To inspect add this spying force to any of the "Force" examples:

function forceSpy(): any {
  function force(alpha: number) {
    // does nothing
  }

  force.initialize = function (_: any) {
    console.log('initializing spy force!');
  };

  return force;
}

… and plug it in like this:

<ForceSimulation
  forces={{
    /* ... */
    spy: forceSpy(),
  }}
  /* ... */
>

… then open the console and watch for "initializing spy force!" while resizing the window.

The reason for this is that the examples are passing forces={{ /* ... */ }}, which due to objects having reference semantics in Svelte's reactivity model causes an update as {…} creates a new object instance every time.

In order to avoid the re-initialization of forces a more efficient approach is to store the forces in a variable:

let forces: Record<string, Force<any, any>> = {
  /* ... */
  center: forceCenter(0, 0),
};

function forcesFor(size: { width: number; height: number }): Record<string, Force<any, any>> {
  const centerForce = forces.center as ForceCenter<any>;
  centerForce.x(size.width / 2).y(size.height / 2);
  return forces;
}

… and pass them to the component like this:

<ForceSimulation
  forces={forcesFor({ width, height })}
  /* ... */
>

The forces now get initialized only once and never re-initialized. While the simulation still works as expected.
There is one caveat with this approach though: any new forces added to the cached forces object will be ignored by ForceSimulation. As such for any structural update of forces the cache object would have to get replaced in forcesFor(…). Not ideal. (Update: see comment below for a cleaner solution.)


This is less of a "bug" in ForceSimulation itself and more of a "bug" in the examples. Either way the examples should highlight the performance implications of passing in forces via {…} literal.

@regexident regexident changed the title ForceSimulation keeps redundantly re-initializing all forces any time the chart's size changes in the examples. ForceSimulation keeps redundantly re-initializing all forces any time any of their parameters change Jun 23, 2024
@regexident
Copy link
Contributor Author

regexident commented Jun 23, 2024

See #206 (comment) for a possible fix, making the use of caching of the forces object itself obsolete.

As such with the above fix one could cache individual forces (especially those with expensive initialization):

const cachedCollideForce = forceCollide(collideRadius);

… and then pass them to the component with the familiar {…} syntax without performance penalty:

<ForceSimulation
  forces={{
    // Initialization of center force is O(1), so we should be fine just passing a new one every frame:
    center: forceCenter(size.width / 2, size.height / 2),
    // Initialization of collide force is O(nodes), so we should re-use it:
    collide: cachedCollideForce
  }}
  /* ... */
>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant