Skip to content

Add a separate PHP 5 only file#938

Merged
morrisonlevi merged 2 commits intomasterfrom
levi/build/separate_php5_php
Jul 6, 2020
Merged

Add a separate PHP 5 only file#938
morrisonlevi merged 2 commits intomasterfrom
levi/build/separate_php5_php

Conversation

@morrisonlevi
Copy link
Copy Markdown
Collaborator

@morrisonlevi morrisonlevi commented Jun 29, 2020

Description

In Sandbox cURL (PHP 5) #911 we added another function to dd_init.php and even though this function is needed only on PHP 5 it still increased memory usage on PHP 7. This PR adds a php5.php file that PHP 5 specific functions can go into without affecting PHP 7 much.

This PR also adds output to 2 more of our tests which were segfaulting because there wasn't any output. This is a known issue with the PHP CLI web server that isn't fixable; we have to keep mitigating it until we move off of this SAPI entirely.

Readiness checklist

  • Changelog has been added to the appropriate release draft.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the appropriate release draft.

@morrisonlevi morrisonlevi added the Overhead Relates to latency, CPU, or memory overhead label Jun 29, 2020
@morrisonlevi morrisonlevi added this to the 0.47.0 milestone Jun 29, 2020
@morrisonlevi morrisonlevi force-pushed the levi/build/separate_php5_php branch from 7874849 to 3c4752c Compare June 29, 2020 21:17
@labbati labbati force-pushed the levi/build/separate_php5_php branch from 3c4752c to 3ed95e9 Compare July 1, 2020 13:40
@morrisonlevi morrisonlevi changed the title Add a separate PHP 5 only file; combine in preloader Add a separate PHP 5 only file Jul 2, 2020
@morrisonlevi morrisonlevi marked this pull request as ready for review July 2, 2020 15:10
SammyK
SammyK previously approved these changes Jul 2, 2020
Copy link
Copy Markdown
Contributor

@SammyK SammyK left a comment

Choose a reason for hiding this comment

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

Nice solution @morrisonlevi!

If the builtin webserver does not get output then it will sometimes
sigsegv. We've hit this case many times. I think the only way we
can stop hitting this case is to not use the built-in CLI webserver.
@morrisonlevi morrisonlevi force-pushed the levi/build/separate_php5_php branch from 58fcea6 to f667076 Compare July 2, 2020 23:11
@morrisonlevi morrisonlevi merged commit bffc93b into master Jul 6, 2020
@morrisonlevi morrisonlevi deleted the levi/build/separate_php5_php branch July 6, 2020 20:57
@morrisonlevi morrisonlevi mentioned this pull request Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Overhead Relates to latency, CPU, or memory overhead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants