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
Draft: Intersect optimization for large unions #1471
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -422,7 +422,7 @@ public function dataCondition(): array | |
new Variable('foo'), | ||
new Variable('bar'), | ||
), | ||
['$foo' => 'Bar', '$bar' => 'Bar'], | ||
['$foo' => 'Bar', '$bar' => 'mixed'], // could be '$bar' => 'Bar' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is really annoying to not have anymore :/ I might spend some time on this in the future to bring back. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry, it looked pretty esoteric to me as a way to narrow down types :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can still do it only for objects, or something like that... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it annoys me enough I will find a way to bring it back without a huge performance loss :) |
||
[], | ||
], | ||
[ | ||
|
@@ -1067,7 +1067,7 @@ public function dataCondition(): array | |
), | ||
[ | ||
'$foo' => 'array<string, mixed>', | ||
'array_filter($foo, \'is_string\', ARRAY_FILTER_USE_KEY)' => 'array<string, mixed>', | ||
'array_filter($foo, \'is_string\', ARRAY_FILTER_USE_KEY)' => 'array', // could be 'array<string, mixed>' | ||
], | ||
[], | ||
], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Just out of curiosity, why creating intermediary variables here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherweise these arrays get
sliced
within the loop over and over againThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right, for some reason I haven't seen the
foreach
loop ! Well done.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neclimdul I feel this
array_slice
change is a no-brainer and should be submitted as a seperate PR.the other change breaks the tests atm, so needs further inspection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, please send a microoptimization PR, then we'll discuss the rest.