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

Improve form performance (tracking issue) #855

Open
2 of 5 tasks
Skaiir opened this issue Oct 19, 2023 · 5 comments
Open
2 of 5 tasks

Improve form performance (tracking issue) #855

Skaiir opened this issue Oct 19, 2023 · 5 comments
Labels
backlog Queued in backlog enhancement New feature or request

Comments

@Skaiir
Copy link
Contributor

Skaiir commented Oct 19, 2023

What should we do?

We should conduct an optimization pass for fjs, as there are some performance issues reported when dealing with large forms.

Why should we do it?

To keep the modeling experience as fluid as possible.

Sample form available here.

Potential improvements

To research

  • can we improve the evaluation time of FEEL ?
  • can we efficiently cache the result of FEEL evaluation, and if so, would performance improve ?
  • can we reduce the overall number of re-renders by using signals ?

To implement

@Skaiir Skaiir added enhancement New feature or request spring cleaning Could be cleaned up one day labels Oct 19, 2023
@pinussilvestrus pinussilvestrus added the backlog Queued in backlog label Oct 20, 2023
@Skaiir Skaiir self-assigned this Oct 23, 2023
@Skaiir
Copy link
Contributor Author

Skaiir commented Oct 26, 2023

Internal: started a chat with modeler team [here]

@barmac
Copy link
Member

barmac commented Oct 26, 2023

I checked the form you shared and indeed it hangs the form playground for a while. Still, the regex looks OK, and a single evaluation does not take that much time on my machine. So it's more a "death by a thousand paper cuts", i.e. that particular function is executed too often. Note that the normalizeContextKey function is called within the context of conditional fields. Perhaps we could save some executions if we confirm that the condition mechanism is causing the problem.

@Skaiir
Copy link
Contributor Author

Skaiir commented Dec 1, 2023

This is still ongoing.

This issue here was raised regarding performance in tasklist for big forms. https://github.com/camunda/tasklist/issues/3758

Following investigations, the performance issues are due to the following:

  • feel evaluations are a bit slow, as mentioned above there have been some investigations into feel noticing nothing specific, but I still think something's going on there, nevertheless we can leave that for later
  • we were overly recalculating filtered state by having the computation within a hook, as part of dynamic list, we're moving this to a context which should reduce calls to feel and make our life easier. This has already improved performance significantly but not to the level that we would like long term
  • we are not caching expressions in any way, so we need to recalculate them all every time the state changes, which could be one avenue where we could make improvements to the performance
  • finally, we could only recalculate the feel expressions whenever the specifically mentioned state objects are updated. This requires us to leverege getSchemaVariables (or a smaller version of it). Might be a bit complex and bug prone, but it would make our life a lot easier
  • or alternatively to that, we could have smarter state management. Using one monolithic object for state has plenty of issues, there are quite a few mechanisms we could put in place to fix that, generally this would be a large refactor but it would bring some improvements beyond just feel across the entire application

@Skaiir Skaiir removed the spring cleaning Could be cleaned up one day label Dec 1, 2023
@Skaiir Skaiir added in progress Currently worked on and removed backlog Queued in backlog labels Dec 11, 2023
@Skaiir Skaiir changed the title Conduct performance pass Improve form performance (tracking issue) Dec 11, 2023
@marcosgvieira
Copy link

@Skaiir as we released improvements for the performance on form-js in our latest release, should we close this one or is there something else we plan to execute?

@Skaiir
Copy link
Contributor Author

Skaiir commented Jan 12, 2024

@marcosgvieira I will unassign myself but keep this issue open in the backlog for future improvements. We need to wait on feedback but generally there are still things that could be improved :D

@Skaiir Skaiir added backlog Queued in backlog and removed in progress Currently worked on labels Jan 12, 2024
@Skaiir Skaiir removed their assignment Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Queued in backlog enhancement New feature or request
Development

No branches or pull requests

4 participants