Performance issues on forms with lists of > 150 items

[gdorsi]

Hello!

Thanks for building this awesome library!

I love expressing the validation using a test-suite declaration style ❤️

I'm using it on a form with 3 lists of items, and when the total number of items start surpassing the 150 elements the computation time of every validation becomes > 100ms (measurement done on my laptop, a Mac M2 Pro)

This is a screenshot of the profiling of a keystroke on that form:

Are there some best practices I can follow in order to avoid incurring into these kind of performance issues?

While I guess it's normal for performance to degrade as the number of fields to validate increases, it feels that the most of the cost comes from the nesting of the various helpers (each, omitWhen, skip, only etc...) so I'm kinda wondering if there are some optimization opportunities lurking inside these utilities.

To reproduce the issue I've create this repro: https://stackblitz.com/edit/vitejs-vite-gwp4ig

Given the simplicity of the example I needed to spawn more items in order to reproduce the same performance issues.

EDIT: More complex repro here --> https://stackblitz.com/edit/vitejs-vite-mlpchh

[ealush]

Thank you for this detailed issue and repro!

I think that’s going to be my weekend project this week. Let me have a deep look into it, and hopefully I can come up with something smart.

I’ll try to see either whether the work can be reduced internally, or if we can come up with some more interesting ideas of utilizing vest's building blocks to achieve the same thing. On Thu, 18 Jul 2024 at 17:29 gdorsi @.***> wrote:

Hello!

Thanks for building this awesome library!

I love expressing the validation using a test-suite declaration style ❤️

I'm using it on a form with 3 lists of items, and when the total number of items start surpassing the 150 elements the computation time of every validation becomes > 100ms (measurement done on my laptop, a Mac M2 Pro)

This is a screenshot of the profiling of a keystroke on that form: Screenshot.2024-07-17.at.16.09.07.png (view on web) https://github.com/user-attachments/assets/d1e59677-aff8-482b-acae-46deb5f1c443

Are there some best practices I can follow in order to avoid incurring into these kind of performance issues?

While I guess it's normal for performance to degrade as the number of fields to validate increases, it feels that the most of the cost comes from the nesting of the various helpers (each, omitWhen, skip, only etc...) so I'm kinda wondering if there are some optimization opportunities lurking inside these utilities.

To reproduce the issue I've create this repro: https://stackblitz.com/edit/vitejs-vite-gwp4ig

Given the simplicity of the example I needed to spawn more items in order to reproduce the same performance issues.

— Reply to this email directly, view it on GitHub https://github.com/ealush/vest/issues/1145, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACV32P7DOPIHZJKRLAFGZIDZM7GLFAVCNFSM6AAAAABLCZYW2CVHI2DSMVQWIX3LMV43ASLTON2WKOZSGQYTMNJWHE4DONA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[gdorsi]

Thanks @ealush :heart:

Let me know if I can be of help

[ealush]

Hey @gdorsi I just went to check the repro blitz and it seems to only contain one field. Did you maybe create in in a fork?

[gdorsi]

Hey @gdorsi I just went to check the repro blitz and it seems to only contain one field. Did you maybe create in in a fork?

Ah sorry, it was broken due to a typo on querySelector!

It is one input, but the validation runs on 500 fields.

I can try to put toghether something more complex if you need a more realistic testing ground.

[gdorsi]

Here is a more complex test, with a mix of each and omitWhen. https://stackblitz.com/edit/vitejs-vite-mlpchh

Here I've managed to get similar performance values to what I'm experiencing in my app.

[ealush]

Thank you for the fixed repro @gdorsi !

I started mapping out the inefficiencies. Found three main inefficiencies so far, and I have taken care of the first one. Not done yet, and it is probably going to be the smaller cost saver of this changeset - but you can already start trying it out in this version: vest@5.4.0-dev-20240719-ed80

Can you let me know if that works any better for you?

[ealush]

Ok, so it took some fiddling, but I think I was able to come up with something that can somewhat improve the performance of large forms.

Here is the new version number you can install: vest@5.4.0-dev-20240720-002e Right off the bat, it should shave off some significant time. It shaved off 40% in my test. I can probably squeeze some more perf out of it, but I will need to find where the rest is coming from. In the meantime, let me know if that works for you. If it does, I'll wrap it nicely and publish a production build.

Here's a the profile result of a simple form with 500 inputs with the production build

And here is the same app with my new development build

[ealush]

Made some more progress on this issue, and now I am able to get results of anywhere between 35ms to 55ms for a 500 fields form.

This latest version can be tried using: vest@5.4.0-dev-20240720-253c All the tests seem to pass, so I would consider this a major win.

I think there are a few more performance gains we can make, especially around internal cache invalidation on each test run. Seems like we're invalidating the cache multiple times during the lifecycle of each tests while we shouldn't really. It is minor (.3ms), but over hundreds of tests it adds up:

[gdorsi]

That's awesome @ealush!

Tested vest@5.4.0-dev-20240720-253c on my app and got around 60% improvement!

Before	After

[ealush]

Ok, one last update @gdorsi I think I was able to get a another perf boost on this:

vest@5.4.0-dev-20240721-db6f

Should reduce response time by a few more ms. I was able to get down all the way from 127 to 35.

Lmk if you experience the same

[gdorsi]

In my case vest@5.4.0-dev-20240721-db6f has the same performance than the version before (vest@5.4.0-dev-20240720-253c)

[ealush]

@gdorsi Thanks for all the feedback!

I created #1146 Tomorrow I will check once again, and if everything looks good on my end, I will publish 5.4.0.

[ealush]

Looks like me committing the diff yesterday closed the issue. Sorry about this.

I just released 5.4.0. Can you please confirm this works as expected on your end?

[gdorsi]

Thanks a mill!

Can confirm that the 5.4.0 lands a huge improvement on performance, our test suite pass and and I've not observed any regression so far.

Tested on the same form but with a different number of fields.

Test on form with 60 fields:
Before	After

Test on form with 180 fields:
Before	After

For now this is a great result as I don't think that there are too many forms of > 100 fields in the wild and the performance with < 100 fields now is above the 60FPS.

Still wondering why the performance degradation isn't linear on the form size. Will dig more into it on my free time to see if I can find some other performance opportunities.

[ealush]

I've got a pretty good idea why this is not linear with the amount of fields. The reason is that for each of the fields, there are some operations that traverse the entire tree, so treeSize*treeSize.

I'm looking for a way to reduce the needs of these as well, because we might not always need this kind of traversal, but at least for now - we were able to get some significant improvement. The rest will require some rearchitecting of the test function, which I am looking into.

For now, I am closing this issue. Thanks for helping me with the fast feedback loop!

[gdorsi]

Ah good!

Thanks for the very quick fix!

[paulodiogo]

I'm facing the same issue. I have a dynamic form that works well with around 100 fields (planning to go 10000+), but when I try a larger form, the performance gets worse. I created theses suites for each part of the form when I'm dealing with them individualy.

https://stackblitz.com/edit/vitejs-vite-sresjq?file=tests%2Ffields.validation.test.js

Created two test files:

fields.validation.not.nested.test.js NN time1: 2.635ms NN time10: 2.285ms NN time100: 15.97ms NN time500: 120.41ms

fields.validation.test.js (nesting the suite) NESTED time1: 2.435ms NESTED time10: 4.175ms NESTED time100: 222.195ms NESTED time500: 15.656s

Note that fields.validation.js I use a inner.fields.validation.js suite.

[ealush]

Hey @paulodiogo, thanks for reporting this and creating this detailed test case scenario. I do indeed see the issue, and I believe it is related to what I replied above to @gdorsi about the increase not being linear.

One thing I think you can do, and still achieve everything you need is to nest via functions rather than suites.

Currently your suite looks like this:

const suite1 = create(() => {/*...*/})

const suite2 = create((data) => {
  suite1(data)
})

If you just replace it with:

function nestedTests1(data) {
  test(/*...*/);
}

const suite = create((data) => {
  nestedTests1(data)
})

You should see an immediate perf boost, without losing any capability. I do want to improve the performance of nested suites (as I mentioned above), but that would require making some changes to the way the test function works, and will require more time.

Will such a solution work for you in the meantime?

[paulodiogo]

Yeah! This change improved the time! Thank you!

From 23secs to 300ms @ealush

[ealush]

@paulodiogo, @gdorsi I made yet another improvement to Vest's infra that should further reduce the initialization time of large suites. Can you try Vest@5.4.1 and let me know how that works?

This should do the trick: https://github.com/ealush/vest/commit/0462bc0d22449029e0522648db126776c3dba2aa

[gdorsi]

@paulodiogo, @gdorsi I made yet another improvement to Vest's infra that should further reduce the initialization time of large suites. Can you try Vest@5.4.1 and let me know how that works?

This should do the trick: https://github.com/ealush/vest/commit/0462bc0d22449029e0522648db126776c3dba2aa

Awesome!

I'm on vacation now and won't be able to give you a feedback before next Monday.

[ealush]

No problem, enjoy your time off!

On Wed, 7 Aug 2024 at 21:28 gdorsi @.***> wrote:

@paulodiogo https://github.com/paulodiogo, @gdorsi https://github.com/gdorsi I made yet another improvement to Vest's infra that should further reduce the initialization time of large suites. Can you try @.*** and let me know how that works?

This should do the trick: 0462bc0 https://github.com/ealush/vest/commit/0462bc0d22449029e0522648db126776c3dba2aa

Awesome!

I'm on vacation now and won't be able to give you a feedback before next Monday.

— Reply to this email directly, view it on GitHub https://github.com/ealush/vest/issues/1145#issuecomment-2274075830, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACV32P43A4QBQJ2CBB52FLDZQJRNTAVCNFSM6AAAAABLCZYW2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZUGA3TKOBTGA . You are receiving this because you were mentioned.Message ID: @.***>

[gdorsi]

@ealush

v.5.4.1 brings another huge performance improvement in our form 🎉

v5.4.0	v5.4.1

Test done editing a single field on a big form of 100 fields

Thanks!

[ealush]

Yes! Glad to hear that this worked for you. Closing the issue now as we extracted every last drop of it.

Thanks for testing reporting back each time.

[ealush]

Also, this is now on twitter https://x.com/evyataral/status/1823776130379272684