Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reduce excessive stack reserve for testing in CI #17166

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Dec 15, 2024

For some reason the stack reserve of php.exe and php-cgi.exe is very large on Windows (64MB)[1]. While this might not be bad for production purposes, it causes stack_limit_014.phpt to be unbearably slow; the test may easily run for a minute, and due to a recent stream_select() improvement[2], that can cause a timeout, what triggers the test to be run again. So this single test case may run for two minutes, and still might fail (happened a couple of times).

Instead of skipping the test in CI, we reduce the stack reserve to 8MB, what improves the performance of this test case (and maybe others), and should still be good enough for CI.

[1] 54906c7
[2] b614b4a

For some reason the stack reserve of php.exe and php-cgi.exe is very
large on Windows (64MB)[1].  While this might not be bad for production
purposes, it causes stack_limit_014.phpt to be unbearably slow; the
test may easily run for a minute, and due to a recent `stream_select()`
improvement[2], that can cause a timeout, what triggers the test to be
run again.  So this single test case may run for two minutes, and still
might fail (happened a couple of times).

Instead of skipping the test in CI, we reduce the stack reserve to 8MB,
what improves the performance of this test case (and maybe others), and
should still be good enough for CI.

[1] <php@54906c7>
[2] <php@b614b4a>
@cmb69
Copy link
Member Author

cmb69 commented Dec 15, 2024

@cmb69 cmb69 marked this pull request as ready for review December 15, 2024 14:37
@cmb69 cmb69 requested a review from TimWolla as a code owner December 15, 2024 14:37
@@ -131,6 +131,10 @@ for %%i in (ldap) do (
del %PHP_BUILD_DIR%\php_%%i.dll
)

rem reduce excessive stack reserve for testing
editbin /stack:8388608 %PHP_BUILD_DIR%\php.exe
Copy link
Member

Choose a reason for hiding this comment

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

just curious is cl.exe able to do it at compile time ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@cmb69 cmb69 merged commit 7e1a241 into php:master Dec 16, 2024
10 checks passed
@cmb69 cmb69 deleted the cmb/stack-reserve branch December 16, 2024 11:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants