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

[3604] Make node overlap resolution faster during "Arrange All" #3635

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

pcdavid
Copy link
Member

@pcdavid pcdavid commented Jun 17, 2024

  • [3604] Update CHANGELOG
  • [3604] Only compute if nodes intersect
  • [3604] Remove useless test
  • [3604] Filter out non-siblings earlier
  • [3604] Avoid creating intermediate rectangles

On the following diagram (a simple diagram with recursive nodes for every papaya::NamedElement, created on the "Package java.lang" element in the "Sirius Web" Papaya example), I go from 60s to a little less than 20s.

image

Note that the first 6s or 7s seem to be spent in ELK? At least during this time I see the loading indicator added in #3574 spinning for these first seconds, and then it stops moving while my CPU is working for the rest of the duration.

The diagram is very specific and does not contain any actual overlap, so it's very possible my changes breaks some stuff. Take with a grain of salt for the moment.

@pcdavid pcdavid linked an issue Jun 17, 2024 that may be closed by this pull request
@pcdavid pcdavid added this to the 2024.7.0 milestone Jun 17, 2024
@pcdavid pcdavid requested a review from frouene June 17, 2024 15:27
@pcdavid
Copy link
Member Author

pcdavid commented Jun 17, 2024

After this first batch, a profiling in Chromium shows this:

Capture d’écran du 2024-06-17 17-40-19

  • First (in red), about 7s waiting for the backend for the result of the mutation layoutDiagram() operation.
  • Then (in blue), about 13s trying to detect and resolve overlaps (which do not exist in this case).

I'm wondering if the backend could pre-compute some of the required information? Even just a flag indicating if there are any overlap so that the front could simply avoid all of this in the (common?) case where there are none.

@frouene frouene force-pushed the pcd/opt/overlaps branch from a0ae3b8 to a4953b7 Compare June 18, 2024 09:20
@frouene frouene marked this pull request as ready for review June 18, 2024 12:25
@frouene frouene requested a review from gcoutable as a code owner June 18, 2024 12:25
@frouene frouene force-pushed the pcd/opt/overlaps branch 2 times, most recently from a35e691 to d3520cc Compare June 19, 2024 06:41
Bug: #3604
Signed-off-by: Pierre-Charles David <[email protected]>
Signed-off-by: Florian ROUËNÉ <[email protected]>
@frouene frouene force-pushed the pcd/opt/overlaps branch from d3520cc to 9371720 Compare June 19, 2024 07:13
@frouene frouene merged commit 9ebb240 into master Jun 19, 2024
3 of 4 checks passed
@frouene frouene deleted the pcd/opt/overlaps branch June 19, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arrange All is extremely slow (unusable) on large diagrams
2 participants