-
Notifications
You must be signed in to change notification settings - Fork 550
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
Optimise foreach on builtin::indexed #22641
base: blead
Are you sure you want to change the base?
Conversation
This change does have the potential impact that, before it, any modifications to the iterated-on array that happen during the body of its own |
…e OP_ENTERITER and not OP_ITER
bf454f3
to
03618f2
Compare
@@ -347,6 +347,30 @@ package FetchStoreCounter { | |||
is(prototype(\&builtin::indexed), '@', 'indexed prototype'); | |||
} | |||
|
|||
# indexed + foreach loop optimisation appears transparent |
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.
commit message "optimisastion" too many s even for British spelling
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.
Ahyes, fixed + forcepushed
…cals Rather than generating an entire temporary list that is twice as big as the original array, instead set a flag on the `OP_ITER` that tells it to set one of the iteration variables to the current array index and use the same `CXt_LOOP_ARY` optimisation that regular foreach over an array would use.
03618f2
to
f3d8512
Compare
Rather than generating an entire temporary list that is twice as big as the original array, instead set a flag on the
OP_ITER
that tells it to set one of the iteration variables to the current array index and use the sameCXt_LOOP_ARY
optimisastion that regular foreach over an array would use.Currently this is a work-in-progress:
Find a neater solution to the
extern void ...
declaration ofXS_builtin_indexed
at the top ofop.c
Consider whether
builtin::indexed LIST...
should do the same optimisationWrite perldelta
This set of changes requires a perldelta entry, and it is not included but I'll write something before un-drafting