Restructure argument passing onto stack #1679
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -70,7 +70,7 @@ ZEND_API int zend_get_parameters(int ht, int param_count, ...) /* {{{ */ | |||
| ZVAL_COPY_VALUE(param_ptr, &new_tmp); | |||
| } | |||
| *param = param_ptr; | |||
| param_ptr++; | |||
| param_ptr--; | |||
There was a problem hiding this comment.
We should remove deprecated API by separate commit: zend_get_parameters(), zend_get_parameters_ex(), zend_copy_parameters_array().
|
The patch is too big for github. zend_vm_def.h changes don't fit :( |
ea9ee86 to
8210746
Compare
Arguments now are prepended to the execute_data, inside the (last!) temporaries of the previous stack frame. Every stack frame has place for sent arguments plus five optional args (RECV_INIT...) and the execute_data. This allows EG(vm_stack_top) and EX(call) to be removed and saves us from expensive comparisons to push a call frame completely in some cases (every ICALL). The only downside is that arguments now are in reverse order causing variadics to be in reverse order on the stack; extensions might need some update ... Also, freeing of args and execute_data is now completely handled by liveness. Finally, the patch gives 0.5%-3% improvement on applications. There likely is still some room for optimization (e.g. eliminating ZEND_SEND_VAL with TMPs or speeding up the unpacking, removing INIT_FCALL for UCALLs, ...).
8210746 to
89c2585
Compare
* master: Fixed incorrect exception handling Align NEWS entry format Align NEWS entry format Align news entry format: Implement FR -> Implemented FR Align news entry format: Implement FR -> Implemented FR Update NEWs Fixed bug #71127 (Define in auto_prepend_file is overwrite) Fix proto and arg info Implemented FR #48532 (Allow pg_fetch_all() to index numerically). Implemented FR #31021 (pg_result_notice() is needed to get all notice messages). Remove sqlite extension leftover references (was removed in PHP 5.4) Fix ZTS build Fix naming
c9f6738 to
98d9614
Compare
|
I'm closing this for now, as there don't seem to be plans to land this in the current form. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Arguments now are prepended to the execute_data, inside the (last!) temporaries of the previous stack frame. Every stack frame has place for sent arguments plus five optional args (RECV_INIT...) and the execute_data.
This allows EG(vm_stack_top) and EX(call) to be removed and saves us from expensive comparisons to push a call frame completely in some cases (every ICALL).
The only downside is that arguments now are in reverse order causing variadics to be in reverse order on the stack; extensions might need some update ...
Also, freeing of args and execute_data is now completely handled by liveness.
Finally, the patch gives 0.5%-3% improvement on applications.
There likely is still some room for optimization (e.g. eliminating ZEND_SEND_VAL with TMPs or speeding up the unpacking, removing INIT_FCALL for UCALLs, ...).