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 Issue: Frequently re-compiling (sub)schemas in AJV #1961

Closed
Viswanathan24 opened this issue Aug 3, 2020 · 23 comments
Closed

Performance Issue: Frequently re-compiling (sub)schemas in AJV #1961

Viswanathan24 opened this issue Aug 3, 2020 · 23 comments
Labels
p1 Important to fix soon performance

Comments

@Viswanathan24
Copy link

i am fetching formData from MongoDB and loading it as initial data, and allowing user to modify, it's like updating existing data,in that i could notice some performance issue when the Jsonschema have more no.of array of items then the browser is crashing and it is not allowing to make any changes for more than 20 mins. can some one help me on this.

Example data:-
image
in above image ruleSteps as 106 elements were finding difficulty to display the form with existing data.

i am using react-jsonschema-form version 1.8.1, Please let me know if any further details is required to troubleshoot the issue

@epicfaace
Copy link
Member

epicfaace commented Aug 3, 2020

@Viswanathan24 can you post your schema and the form data? Ideally a link to a playground example would be best.

@Viswanathan24
Copy link
Author

@epicfaace Please find the link below and help me to resolve it.
PlayGroud Link

@epicfaace
Copy link
Member

My browser doesn't crash on that example, though it is a bit slow to add / remove / modify items.

@Viswanathan24
Copy link
Author

image
Please provide me the browser details and any kind of tips and tricks to avoid browser crash then that should help

@AlimovSV
Copy link
Contributor

AlimovSV commented Apr 29, 2022

I faced with this issue too. This performance issue appears when your schema is large enough and has a lot of if/then/else or anyOf/allOf conditions. I realized that bottle neck here: https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/core/src/validate.js#L307-L308 so this code means that on every keystroke we make AJV parsing rootSchema. This is is a large overhead. After patching this method with something like:

const validators = new Map();

export function isValid(schema, data, rootSchema) {
  let validator = validators.get(rootSchema);

  if (!validator) {
    validator = createAjvInstance()
      .addSchema(rootSchema, ROOT_SCHEMA_PREFIX);

    validators.set(rootSchema, validator);
  }

  try {
    // add the rootSchema ROOT_SCHEMA_PREFIX as id.
    // then rewrite the schema ref's to point to the rootSchema
    // this accounts for the case where schema have references to models
    // that lives in the rootSchema but not in the schema in question.
    return validator.validate(withIdRefPrefix(schema), data);
  } catch (e) {
    return false;
  }
}

any performance issues were gone. But you need to assure that you do not pass schema into the form as a new object every time (something like <Form schema={{ type: 'object' }} /> because in such a case this method will create a new AJV instance on every keystroke (and memory will leak without increase performance).

@AlimovSV
Copy link
Contributor

AlimovSV commented May 4, 2022

@nickgros Please take a look at this proposed change and share your feedback.

@nickgros
Copy link
Contributor

nickgros commented May 4, 2022

@AlimovSV I tried to tackle this some time ago with tons of memoization, and mapping schemas to compiled validators: #2466. I had the same takeaway as you finding that conditional behavior ends up triggering expensive synchronous compilation in AJV, and we end up not taking advantage of its built-in cache.

Your solution is much more simple than mine! If it significantly improves performance without causing any regressions, I think it would be worth adding. We could add a note in the docs or come up with some other solution to warn users to memoize their schema.

Tangentially related, @heath-freenome is planning on externalizing validation in #2693. Perhaps during/after that change we could take advantage of asynchronous compilation/validation.

lpillonel added a commit to lpillonel/react-jsonschema-form that referenced this issue Jun 2, 2022
@lpillonel
Copy link
Contributor

We also faced exactly this, and @AlimovSV solution is really solving the performance issue for us.
if #2693 is not planned for next release, it might be a quick win in the meantime.

@nickgros
Copy link
Contributor

nickgros commented Sep 9, 2022

#2693 is done, so we would be happy to accept a PR in the validator/ajv-8 repo in v5 @lpillonel @AlimovSV

@nickgros nickgros changed the title Performance Issue Performance Issue: Frequently re-compiling (sub)schemas in AJV Sep 9, 2022
@heath-freenome heath-freenome added the p1 Important to fix soon label Sep 23, 2022
@adefrutoscasado
Copy link

adefrutoscasado commented Feb 1, 2023

I am facing this issue too. My schema has a lot of if/then/else and anyOf/allOf condition.

Is there any workaround or beta version that includes the change?

Love this package! 👌

@zbayoff
Copy link

zbayoff commented Sep 5, 2023

We are facing this issue with our large schema and conditional fields. Typing in inputs is a bit laggy, especially when more array items are added. It would be great to have the above mentioned solution implemented in the package.
Thanks again for all your hard work on this excellent package!

@heath-freenome
Copy link
Member

Theoretically this was fixed with #3721. Sounds like maybe it didn't work?

@igorbrasileiro
Copy link
Contributor

I'm encountering the same issue as well. When profiling the performance using Chrome DevTools, specifically with a large schema that involves anyOf and allOf properties, and with live validation enabled, I noticed that when pressing a single key, the compileSchema function is matched 343 times in the flame graph.

image

@magaton
Copy link

magaton commented Oct 30, 2023

This is a major problem for me as well. I have lots of allOf + if-then constructs with string in array conditions in my schema.
I tried assigning $id to the root schema, but that did not help.
Could maintainers suggest something, please?

@heath-freenome
Copy link
Member

@cwendtxealth Is this what you were talking about at today's meeting? Could you poke at this issue and see if you can solve the performance issue?

@magaton have you tried precompiled schemas?

@johannes-sauer-constellr

I think I am experiencing the same problems here as well, I do have a rather big schema with including a large allOf filled with if/then (conditional sub schemas). I sifted through the flame graph of one keystroke in a text field of the generated form and saw many time-consuming calls to retrieveSchema.

In my case it seems like I am able to eliminate the calls to retrieveSchema, meaning that I simply use the original schema that would be passed as an argument to this function. This might be specific to my schema, as it does not use $ref, and has no recursion, so maybe it is already of the expected form. Also I precompile my schema, but tbh I don't understand the relation to precompiling here yet. In any case, I think it looks a bit like expensive calls to retrieveSchema could be saved also for the general case, e.g. by caching its result.

Please take everything I wrote with a grain of salt, I'm not at all experienced with Javascript, React, nor rjsf. So please excuse me if my idea does not make any sense. Thanks a lot for putting the work into this component, it is really handy for us.

@igorbrasileiro
Copy link
Contributor

I tried assigning $id to the root schema, but that did not help.

I'm using the schema with this shape below, and the $id helped me to avoid recompile schema

{
  "definitions": {},
  "root": {},
  "$ref": "#/definitions/something",
 "$id": 312321321 // this is Date.now(). I'm doing this because, my use case can update the schema.
}

@heath-freenome
Copy link
Member

Yeah, retrieveSchema gets called in smaller and smaller chunks to deal with $refs and resolving anyOf/allOf/oneOf schemas for rendering. I'm not sure how we might be able to reduce the number of calls off of the top of my head.

@igorbrasileiro
Copy link
Contributor

Yeah, retrieveSchema gets called in smaller and smaller chunks to deal with $refs and resolving anyOf/allOf/oneOf schemas for rendering. I'm not sure how we might be able to reduce the number of calls off of the top of my head.

After looking at the problem again it looks to me like additionally to the fix of #3970, another call of retrieveSchema could be eliminated: in toIdSchema there is a (conditional) call of retrieveSchema, even though in all places that toIdSchema is called, its schema arg is already the result of a call to retrieveSchema with the same args as the internal call.

For my schema these redundant calls get quite expensive. I'll try to implement a fix as a PR.

Also the field schema could get the schema from the form, that the Form render

@johannes-sauer-constellr

I deleted the comment, because I didn't think through the part where toIdSchemaInternal is called recursively with different schema args. I still think it might be enough to call retrieveSchema once before instead of during the recursion. Will need to see if that works in general or just for my schema.

@ani9450
Copy link

ani9450 commented Dec 7, 2023

I faced with this issue too. This performance issue appears when your schema is large enough and has a lot of if/then/else or anyOf/allOf conditions. I realized that bottle neck here: https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/core/src/validate.js#L307-L308 so this code means that on every keystroke we make AJV parsing rootSchema. This is is a large overhead. After patching this method with something like:

const validators = new Map();

export function isValid(schema, data, rootSchema) {
  let validator = validators.get(rootSchema);

  if (!validator) {
    validator = createAjvInstance()
      .addSchema(rootSchema, ROOT_SCHEMA_PREFIX);

    validators.set(rootSchema, validator);
  }

  try {
    // add the rootSchema ROOT_SCHEMA_PREFIX as id.
    // then rewrite the schema ref's to point to the rootSchema
    // this accounts for the case where schema have references to models
    // that lives in the rootSchema but not in the schema in question.
    return validator.validate(withIdRefPrefix(schema), data);
  } catch (e) {
    return false;
  }
}

any performance issues were gone. But you need to assure that you do not pass schema into the form as a new object every time (something like <Form schema={{ type: 'object' }} /> because in such a case this method will create a new AJV instance on every keystroke (and memory will leak without increase performance).

@AlimovSV , Can you please suggest how do i prevent passing the schema as a new object from parent to child component (which has rjsf form) ? as i am storing schema in parent state and on passing to child, the child will have new object.

Any suggestions would be highly appreciated !

Thanks

@AlimovSV
Copy link
Contributor

AlimovSV commented Dec 7, 2023

@ani9450 First of all, I would say that this ticket is obsolete and RJSF team made a lot of work to cover the case with slow performance (big thanks all of them!). Regarding schema, unfortunately the passing of the new schema object into the child component is a by-design decision (because a schema for child component may depends on the current form state and have to be resolved (this means = create new object).

@heath-freenome heath-freenome added possibly close To confirm if this issue can be closed and removed help wanted labels Dec 8, 2023
@stale stale bot removed the possibly close To confirm if this issue can be closed label Dec 8, 2023
@heath-freenome
Copy link
Member

I am closing this ticket as there is a newer, more relevant ticket related to performance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1 Important to fix soon performance
Projects
None yet
Development

No branches or pull requests