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

[7.x] Performance improvements #32532

Closed
wants to merge 2 commits into from

Conversation

halaei
Copy link
Contributor

@halaei halaei commented Apr 24, 2020

This PR improve asymptotic algorithmic complexity of validation, the speed-up is more than a constant number and it depends on the input size.

Validation tests are passed in my system. I don't know if they break anything. It should be backported to LTS versions. I don't regard this as a breaking change, but you may decide to choose portion of it for version 7, 6, and 5.5.
The performance may still be poor in some cases. But other improvements may require breaking changes or bigger changes.

@driesvints driesvints changed the title Performace improvements [7.x] Performance improvements Apr 24, 2020
@taylorotwell
Copy link
Member

Missing doc blocks... also, "I don't know if this breaks anything" doesn't inspire confidence.

@halaei halaei force-pushed the performace-improvements branch from f36f155 to 73c290e Compare April 24, 2020 16:46
@staudenmeir
Copy link
Contributor

There's a reason for removing duplicate keys: #4126

@halaei
Copy link
Contributor Author

halaei commented Apr 24, 2020

I added some phpdocs.
It is not easy to reverse engineer and understand the code. It took me 2 days so far. I think my changes are good. No guarantee because it is complicated.

@halaei
Copy link
Contributor Author

halaei commented Apr 24, 2020

@staudenmeir I remove my changes on Relation. That has nothing to do with validation after all. Maybe Collection::unique() can be improved later. It takes O(n^2) time because the call to in_array inside reject().

@halaei halaei force-pushed the performace-improvements branch from 73c290e to d89c9fd Compare April 24, 2020 17:11
@GrahamCampbell
Copy link
Member

GrahamCampbell commented Apr 24, 2020

5.5.

5.5 is no longer maintained. LTS support for it already ended. Everyone should upgrade to at least Laravel 6.

@taylorotwell
Copy link
Member

There are changes here that appear breaking and I can't risk that on a point release. If you are concerned about people overwhelming your application check the size of the array before sending it to the validator.

@halaei
Copy link
Contributor Author

halaei commented Apr 26, 2020

So maybe Laravel 8

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

Successfully merging this pull request may close these issues.

4 participants