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

Performance issues when quickly updating cubic bezier values #18

Open
kitschpatrol opened this issue Nov 14, 2023 · 2 comments
Open

Performance issues when quickly updating cubic bezier values #18

kitschpatrol opened this issue Nov 14, 2023 · 2 comments

Comments

@kitschpatrol
Copy link
Contributor

kitschpatrol commented Nov 14, 2023

Thank you for this superb library!

I noticed creeping memory usage when repeatedly setting the value of a cubicbezier blade.

A minimal (but admittedly extreme) reproduction is something like this. After about five minutes, I see several hundred megabytes of usage in the tab.

Maybe CubicBezier value objects are being retained somewhere internally? Manually invoking GC helps a bit but still doesn't completely stop the accumulation.

<script type="module">
  import * as Tweakpane from 'https://unpkg.com/[email protected]/dist/tweakpane.js';
  import * as EssentialsPlugin from 'https://unpkg.com/@tweakpane/[email protected]/dist/tweakpane-plugin-essentials.js';

  const pane = new Tweakpane.Pane({ title: 'Cubic Bezier Stress Test' });
  pane.registerPlugin(EssentialsPlugin);

  const bezierApi = pane.addBlade({
    view: 'cubicbezier',
    value: [0, 0, 0, 0],
  });

  setInterval(() => {
      bezierApi.value = new EssentialsPlugin.CubicBezier(
        Math.random(),
        Math.random(),
        Math.random(),
        Math.random()
      );
  }, 4);
</script>
@cocopon
Copy link
Contributor

cocopon commented Dec 17, 2023

Thank you for reporting the issue. I've tried to reproduce the issue with your code but I can't.

  • Memory usage of Chrome by Activity Monitor.app didn't increase significantly (297.3MB to 297.5MB in 5mins)
  • Chrome's memory profiler doesn't detect unexpected values
    image

What is your environment? Here is mine:

  • macOS 13.6
  • Google Chrome latest (120.0.6099.109)

@kitschpatrol
Copy link
Contributor Author

kitschpatrol commented Dec 18, 2023

Thank you very much for looking into this — I've updated my browser since I opened the issue, and when re-testing today I'm not able to reproduce any memory accumulation either, so that doesn't seem to be the issue.

I realize I should have explained why I was looking into a possible leak in the first place: I had observed frame rate slow-downs in tabs that have been open for a long time with regularly-updated CubicBezier controls.

My memory leak hypothesis was wrong, apologies — but I investigated the issue a bit further today, and think I found the cause.

If the time between setting the .value on the CubicBezier API is less than the duration of the preview animation (~1400ms?), then the un-cancelled calls to requestAnimationFrame in cubic-bezier-preview.ts keep accumulating and firing, eventually leading to the frame rate slowdown.

Tracking the animation frame ID and then cancelling the last requestAnimationFrame in stop() seems to fix this. I'm happy to share a PR with the change.

Here's a video (of a very exaggerated scenario) that demonstrates the performance difference. The plugin-essentials version 0.2.1 is shown at left, and a fork with requestAnimationFrame cancellation is shown at right. (Tested on the latest Chrome running on macOS 14.2.)

CubicBezier.Performance.Example.mp4

Here's the test code. I'm using a bunch of blades to demonstrate the effect quickly, but even with a single blade the slowdown will eventually occur as long as the value update interval is < 1400ms.

<script type="module">
  import * as Tweakpane from "https://unpkg.com/[email protected]/dist/tweakpane.js";
  import * as EssentialsPlugin from "https://unpkg.com/@tweakpane/[email protected]/dist/tweakpane-plugin-essentials.js";

  const pane = new Tweakpane.Pane({ title: "Cubic Bezier Stress Test 2" });
  pane.registerPlugin(EssentialsPlugin);
  const fpsGraph = pane.addBlade({ view: "fpsgraph" });

  const cbCount = 12;
  const cbs = [];
  for (let i = 0; i < cbCount; i++) {
    cbs.push(
      pane.addBlade({
        view: "cubicbezier",
        value: [0, 0, 0, 0],
        picker: "inline",
        expanded: true,
      })
    );
  }

  function render() {
    fpsGraph.begin();
    for (const cb of cbs) {
      cb.value = new EssentialsPlugin.CubicBezier(
        Math.random(),
        Math.random(),
        Math.random(),
        Math.random()
      );
    }
    fpsGraph.end();
    requestAnimationFrame(render);
  }

  render();
</script>

I'll re-title the issue to better reflect the observed behavior rather than a possible cause.

@kitschpatrol kitschpatrol changed the title Possible memory leak in cubic bezier Performance issues when quickly updating cubic bezier values Dec 18, 2023
kitschpatrol added a commit to kitschpatrol/tweakpane-plugin-essentials that referenced this issue Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants