Skip to content

Conversation

dstogov
Copy link
Member

@dstogov dstogov commented Mar 11, 2015

Instead of throwing zend_error() from signal handler, now we just set EG(vm_interrupt) and EG(timed_out) flags. PHP VM checks EG(vm_interrupt) flag on each JMPx instruction (potential loop iteration) and then throws the same zend_error() from VM context. This is safe, and we don't need to wrap some critical code sections with HANDLE_BLOCK_INTERRUPTIONS/HANDLE_UNBLOCK_INTERRUPTIONS anymore (we will need them only in opcache). A small overhead of checking EG(vm_interrupt) on jumps is counterbanaced by improvement from HANDLE_BLOCK_INTERRUPTIONS/HANDLE_UNBLOCK_INTERRUPTIONS removal.

This mechanism is similar to timeout interrupt handling on Windows, but more efficient.
It doesn't allow interruption of internal functions.

In the future EG(vm_interrupt) may be reused for different purposes.

Instread of throwing zend_error() from signal handler, now we just set EG(vm_interrupt) and EG(timed_out) flags.
PHP VM checks EG(vm_interrupt) flag on each JMPx instruction (potential loop iteration) and then throws the same zend_error() from VM context.
This is safe, and we don't need to wrap some critical code sections with HANDLE_BLOCK_INTERRUPTIONS/HANDLE_UNBLOCK_INTERRUPTIONS anymore (we will need them only in opcache).
A small overhead of checking EG(vm_interrupt) on jumps is counterbalnaced by improvement from HANDLE_BLOCK_INTERRUPTIONS/HANDLE_UNBLOCK_INTERRUPTIONS removal.

This mechanism is similar to timeout interrupt handling on Windows, but more efficient.
It doesn't allow interruption of internal functions.

In the future EG(vm_interrupt) may be reused for different purposes.
@nikic
Copy link
Member

nikic commented Jun 4, 2016

This mostly landed as 650c1c0.

@dstogov This PR also dropped a bunch of HANDLE_BLOCK/UNBLOCK_INTERRUPTIONS. Can we do this for master now, or not?

@dstogov
Copy link
Member Author

dstogov commented Jun 6, 2016

They are empty macros now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants