Skip to content
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

global tick API support #4171

Closed
wants to merge 0 commits into from
Closed

global tick API support #4171

wants to merge 0 commits into from

Conversation

shiguangqi
Copy link
Contributor

Adding this API so we can set a global default ticks number that can be used in every php file, avoiding set declare(ticks=N) at the beginning of each file many times.
Please considering this request, it is so important to us, we have used tick to implemented preemptive scheduling for Swoole Coroutine, it is bad to set declare(ticks=N) in each php file. we can use zend_set_global_tick API in our extension by once with a default value.

@shiguangqi
Copy link
Contributor Author

shiguangqi commented May 16, 2019

I am one of developers of Swoole(https://github.com/swoole/swoole-src) from china, with the request we can implement following, including involves many php files.
php -dswoole.ticks=1000 test.php

<?php
$max_msec = 10;
Co::set(['max_exec_msec' => $max_msec]);

$start = microtime(true);
echo "start\n";
$flag = 1;

go(function () use (&$flag, $max_msec) {
    echo "coro 1 start to loop\n";
    $i = 0;
    while ($flag) {
        $i++;
    }
    echo "coro 1 can exit\n";
});

$end = microtime(true);
$msec = ($end - $start) * 1000;

go(function () use (&$flag) {
    echo "coro 2 set flag = false\n";
    $flag = false;
});
echo "end\n";
?>
--EXPECTF--
start
coro 1 start to loop
coro 2 set flag = false
end
coro 1 can exit

Copy link
Member

@KalleZ KalleZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch has uses spaces instead of tabs, please fix that.

The function body is missing ZEND_API.

The new CG(ticks) should be initialized before use.

@shiguangqi
Copy link
Contributor Author

Thank you so much for your reply, I have modified the request as your help, please help review again.

@shiguangqi
Copy link
Contributor Author

I have updated the request just now and using a static zend_default_ticks instead of CG(ticks), cause CG(ticks) will cause ABI incompatible. Please help review again, Thank you so much!

@shiguangqi shiguangqi force-pushed the master branch 3 times, most recently from 99d1fe9 to 1dc2695 Compare May 17, 2019 02:46
@cmb69
Copy link
Member

cmb69 commented May 17, 2019

Using a global variable would not be thread-safe, would it? An ABI incompatibility due to introducing CG(ticks) in PHP 8 (or even 7.4) would be permissible.

@shiguangqi
Copy link
Contributor Author

@cmb69 I think it is thread-safe, only call zend_set_global_tick on MINIT phase before compile PHP file will be useful. once we set zend_default_ticks value, it would not be modified again. the default value will be effective in all scope except the file using declare(ticks=N) individual.

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.

4 participants