NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Config] Fix files sorting in GlobResource#43591

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:4.4fromlyrixx:config-glob-bug

Oct 19, 2021

Merged

[Config] Fix files sorting in GlobResource#43591

fabpot merged 1 commit intosymfony:4.4fromlyrixx:config-glob-bug

Oct 19, 2021

Conversation

Copy link

Member

lyrixx commentedOct 19, 2021•
edited
Loading

Q	A
Branch?	4.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets
License	MIT
Doc PR

It should be on path and not on info

and it's a big perf boost:https://blackfire.io/profiles/compare/27f409a5-73c7-4285-9b58-e631abbdb325/graph

[Config] Fix files sorting in GlobResource

2d2fe15

It should be on path and not on infoand it's a big perf boost:https://blackfire.io/profiles/compare/27f409a5-73c7-4285-9b58-e631abbdb325/graph

carsonbot added Status: Needs Review Bug Config labels

Oct 19, 2021

carsonbot added this to the4.4 milestone

Oct 19, 2021

nicolas-grekas approved these changes

Oct 19, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Oct 19, 2021

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

that's a bug to me: sorting was supposed to happen on keys, not on values

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I get it, but it's been like that for years.

Copy link

Member

@fabpot I don't think it changes the behavior. casting the SplFileInfo to string (when passing the value tostrnatcmp) produces the same value than the key in the array (but slower). So the behavior should still be the same (and the proof is the fact that tests have not required changes).

For the initial sort, this is an optimisation to try to have an array that is already partially sorted when merging the arrays of different paths. Usingnatsort there is also a bugfix making the optimization actually work (by using the same sorting than in the final array)