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 degradation on the example app #2139

Closed
Janpot opened this issue Jun 6, 2023 · 1 comment · Fixed by #2142
Closed

Performance degradation on the example app #2139

Janpot opened this issue Jun 6, 2023 · 1 comment · Fixed by #2142
Assignees
Labels
performance priority: important This change can make a difference regression A bug, but worse
Milestone

Comments

@Janpot
Copy link
Member

Janpot commented Jun 6, 2023

Steps to reproduce 🕹

Steps:

  1. Run and profile the example application: Example-1: admin app #2096

  2. Notice the app being very sluggish, up to a point we can call it unusable.

  3. Record a performance trace

  4. Notice it renders on every keystroke and the renders take about ~200 - ~250ms to complete (I'm on a Macbook air M1):

    Screenshot 2023-06-06 at 13 32 29

Current behavior 😯

It's just too slow. Did a quick code review and the biggest contributor to this sluggishness seems to be because we are recalculating the state for the whole page on every node we render. This is a regression that was likely introduced when we added nested scopes to the runtime to enable List and Form components.
Before that the whole page used to be calculated on every keystroke, but only once. This is also a bit wasteful, but nothing compared to doing it on every node which turns a O(n) algorithm into O(n^2) (n being the amount of nodes on the page).

Expected behavior 🤔

The page state is only calculated once for the whole page

@Janpot Janpot added priority: important This change can make a difference performance regression A bug, but worse labels Jun 6, 2023
@Janpot Janpot self-assigned this Jun 6, 2023
@Janpot Janpot added this to the beta-critical milestone Jun 6, 2023
@apedroferreira
Copy link
Member

apedroferreira commented Jun 6, 2023

I didn't notice at the time but those changes might have an impact on the performance...
Thanks for looking into it, let me know if I can help with anything!

Edit: also we could try to create some automated checks for performance eventually, it might be useful to prevent this kind of issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance priority: important This change can make a difference regression A bug, but worse
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants