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 problems with allOf #4051

Open
4 tasks done
magaton opened this issue Jan 19, 2024 · 6 comments
Open
4 tasks done

Performance problems with allOf #4051

magaton opened this issue Jan 19, 2024 · 6 comments

Comments

@magaton
Copy link

magaton commented Jan 19, 2024

Prerequisites

What theme are you using?

mui

Version

5.16.1

Current Behavior

When using allOf subschemas are validated multiple times for each render, no matter if the controlling field (where allOf is placed) has been changed or not.
This results in many. in the complete version, hundreds of unnecessary calls of isValid() in validator.ts.

Moreover, in validator.ts, if I remove the comment from this line, the caching of rootSchema is enabled.
This helps a bit since rootSchema is not validated for each render, but subschemas are validated, although compiled only once (initially) since their compiled validator is found in the cache.

I have tried to figure out what is causing this. I have looked in MultischemaField, but could not understand why subschemas are validated for each render, multiple times. Also, I have tried to add a discriminator, hoping that this would skip calling isValid, but no luck, since my controlling fields are usually arrays and allOf is using contains.

My battle with performance is really a long one:

but this time, I think I was able to identify the problem.

It is not a compile problem, compilers are cashed by generated hash, but numerous isValid calls.

Could you please help, at least by pointing out what is causing this and what could be the solution for this problem, so that, at least, users can use my application? Now it is terribly slow and laggy on each keystroke.

Thanks in advance.

Expected Behavior

I would expect that isValid is called only once per each render, if and only if the controlling field value has been changed.

Steps To Reproduce

Please check the repo demonstrating this problem. You will be able to see numerous isValid calls for each subschema in every allOf, no matter whether it is an initial render or change in a completely irrelevant field.

Environment

- OS: mac osx
- Node: latest
- npm: latest

Anything else?

No response

@magaton magaton added bug needs triage Initial label given, to be assigned correct labels and assigned labels Jan 19, 2024
@magaton
Copy link
Author

magaton commented Jan 19, 2024

Could this be a reason?

@heath-freenome
Copy link
Member

@magaton the isValid() has to be called once per condition per render. And it may likely be done more than necessary. I wish I had the time to dig into this. There are so many things that I wish were implemented/fixed. Unfortunately we are only volunteers. How can we help you fix this for yourself?

@heath-freenome heath-freenome added awaiting response and removed needs triage Initial label given, to be assigned correct labels and assigned labels Jan 26, 2024
@magaton
Copy link
Author

magaton commented Jan 29, 2024

@heath-freenome, thanks for the answer.
What would be the idea behind validating allOf conditions every time on each render, no matter what property has been changed?
I would expect that only if the property that drives allOf conditions changed, the allOf conditions are validated.
I think this is how oneOf and anyOf work. Can you confirm that?

It would be very helpful if you could tell me what I need to change locally to eliminate those excessive isValid calls for all allOfsubschemas.

Thanks

@magaton
Copy link
Author

magaton commented Feb 2, 2024

@heath-freenome I have realized it is not about allOf but if-then-else, either standalone or inside allOf.
if-the-else is validated every time, instead ONLY if the controlling field changed its value. I am wondering whether it would be possible to add this feature.
At this point, the only way how I was able to bypass the extensive validation of all (100s) of conditionals in the schema was to set helper fields on the form onChange event which I put inside the dependencies section. No need to say that I ended up with huge number of them + super complicated onChange handler.

@heath-freenome
Copy link
Member

heath-freenome commented Feb 2, 2024

@magaton Feel free to come to the RJSF meeting today at 12pm PST to talk about this

@heath-freenome
Copy link
Member

@magaton or we could chat one-on-one via Discord.

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

No branches or pull requests

2 participants