- 
                Notifications
    You must be signed in to change notification settings 
- Fork 8k
Restructure argument passing onto stack #1679
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
Conversation
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.
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
…_restructuring
c9f6738    to
    98d9614      
    Compare
  
    …_restructuring
| I'm closing this for now, as there don't seem to be plans to land this in the current form. | 
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, ...).