Skip to content

Conversation

@Skaiir
Copy link

@Skaiir Skaiir commented Nov 5, 2025

Closes #81
Related to #96

Proposed Changes

This adds two configs debounceDelay and debounceSkipDelay, which prevent updates from running frequently when many update triggering actions are fired consecutively. In practice this means any updates fired within debounceDelay ms are fired off as a single update at the end UNLESS debounceSkipDelay is reached, which triggers an update regardless to allow for very long running renders to still display a somewhat current mini-map. Note this does not apply to user interactions with the minimap, as debouncing interferes with the viewport visuals (I haven't figured out a good way to handle this yet).

Secondly, while the minimap is closed, we simply do not update it. However we always force a non-debounced update on open (to ensure viewport information is properly initialized).

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached - Can't really display much, but this cuts down the rendering time of 2000 elements in the process landscape from over 5 minutes to under 1.
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Nov 5, 2025
@Skaiir Skaiir force-pushed the 96-minimap-performance branch from f8423a2 to d9982d6 Compare November 5, 2025 22:19
@Skaiir Skaiir marked this pull request as draft November 5, 2025 22:40
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Nov 5, 2025
@Skaiir Skaiir force-pushed the 96-minimap-performance branch from d9982d6 to cb7abf8 Compare November 5, 2025 23:06
@Skaiir Skaiir requested a review from philippfromme November 5, 2025 23:09
@Skaiir Skaiir marked this pull request as ready for review November 5, 2025 23:09
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Nov 5, 2025
@nikku
Copy link
Member

nikku commented Nov 6, 2025

@Skaiir Let's please have a test case for the "extreme setup" that we're targeting. I'd love to ensure this is properly tested.

@Skaiir
Copy link
Author

Skaiir commented Nov 6, 2025

@nikku I've set something up quickly but because it's asynchronous I wasn't able to turn it into a proper unit test. Maybe I need to make it a little less intense though because at 3000+ items it looks like diagram-js starts falling apart.

Edit:

What do you think of this, we have this asynchronous observable extreme extreme case test which we keep as a .skip for dev testing ad-hoc, and we add another slightly less insane synchronous test (no yielding back to the browser, so it will freeze for a while) and expect a maximum execution time. I think if I keep the number of elements drawn around 1500-2000 it's somewhat within 5-10 seconds execution time with the current perf.

@Skaiir Skaiir force-pushed the 96-minimap-performance branch from 52fb0bd to 9e631b0 Compare November 6, 2025 16:12
@Skaiir Skaiir force-pushed the 96-minimap-performance branch from 9e631b0 to 414001f Compare November 6, 2025 16:12
@Skaiir Skaiir marked this pull request as draft November 6, 2025 17:13
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Nov 6, 2025
@nikku
Copy link
Member

nikku commented Nov 17, 2025

What do you think of this, we have this asynchronous observable extreme extreme case test which we keep as a .skip for dev testing ad-hoc, and we add another slightly less insane synchronous test (no yielding back to the browser, so it will freeze for a while) and expect a maximum execution time.

I'd ensure that the minimap does not actually block things, i.e. continuing to model works.

I think if I keep the number of elements drawn around 1500-2000 it's somewhat within 5-10 seconds execution time with the current perf.

Yes, that could be feasible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in progress Currently worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update minimap outside of render cycle

3 participants