Skip to content

Remove unneeded --disable-inline-optimization build parameter #5729

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

Closed
wants to merge 1 commit into from

Conversation

alexdowad
Copy link
Contributor

In 1999, inline optimization was turned off by default. The commit log indicates this was
done because GCC was running out of memory on some hosts when building the Zend executor.
In 2003, inline optimization was re-enabled by default, but a build option was added to
turn it off if one runs out of memory when building.

Computing hardware has come a long way since 2003 and I doubt that anyone is running out
of memory when building PHP now.

Interestingly, this code set an unused variable called INLINE_CFLAGS. It actually
disabled inline optimization by adding -O0 to the build command, not using INLINE_CFLAGS.

Just to see how much memory GCC/Make are using when building PHP, I tried building with
successively higher values of ulimit -v until it succeeded. Interestingly, while most
of the codebase can be built with about 400MB of memory, ext/fileinfo/libmagic/apprentice.c
requires 1.2GB, doubtless because it includes ext/fileinfo/data_file.c, which is more
than 350,000 lines long. That is with GCC 7.5.0.

Most users get PHP as a binary package anyways, so the question is, are packagers
of PHP trying to build on machines with just 1GB RAM? And would they want to package
a PHP interpreter built with no optimizations? I can't imagine either being true.

In 1999, inline optimization was turned off by default. The commit log indicates this was
done because GCC was running out of memory on some hosts when building the Zend executor.
In 2003, inline optimization was re-enabled by default, but a build option was added to
turn it off if one runs out of memory when building.

Computing hardware has come a long way since 2003 and I doubt that anyone is running out
of memory when building PHP now.

Interestingly, this code set an unused variable called `INLINE_CFLAGS`. It actually
disabled inline optimization by adding -O0 to the build command, not using `INLINE_CFLAGS`.

Just to see how much memory GCC/Make are using when building PHP, I tried building with
successively higher values of `ulimit -v` until it succeeded. Interestingly, while most
of the codebase can be built with about 400MB of memory, ext/fileinfo/libmagic/apprentice.c
requires 1.2GB, doubtless because it includes ext/fileinfo/data_file.c, which is more
than 350,000 lines long. That is with GCC 7.5.0.

Most users get PHP as a binary package anyways, so the question is, are *packagers*
of PHP trying to build on machines with just 1GB RAM? And would they want to package
a PHP interpreter built with *no optimizations*? I can't imagine either being true.
@alexdowad alexdowad force-pushed the remove-disable-inline branch from 37909ec to 417ff79 Compare June 17, 2020 08:13
@nikic
Copy link
Member

nikic commented Jun 17, 2020

I agree that this option has very likely outlived it's usefulness. We can still bring it back if someone complains :)

Just to see how much memory GCC/Make are using when building PHP, I tried building with
successively higher values of ulimit -v until it succeeded. Interestingly, while most
of the codebase can be built with about 400MB of memory, ext/fileinfo/libmagic/apprentice.c
requires 1.2GB, doubtless because it includes ext/fileinfo/data_file.c, which is more
than 350,000 lines long. That is with GCC 7.5.0.

This is an interesting point. We should probably try to avoid that. A simple thing to try would be to initialize the array using a string literal, rather than an array. I suspect that will reduce the memory requirements, but that's just a guess.

Of course, the proper way would be to link this in separately based on something like ld -b binary. I'm not sure how good interoperability is in that area though.

@nikic
Copy link
Member

nikic commented Jun 17, 2020

@alexdowad I have granted you commit access to the php-src repo on git.php.net, so you should be able to commit this change directly. There's some git info at https://wiki.php.net/vcs/gitworkflow and https://wiki.php.net/vcs/gitfaq, though it's a bit all over the place. Drop into R11 if you have any questions.

@alexdowad
Copy link
Contributor Author

@alexdowad I have granted you commit access to the php-src repo on git.php.net, so you should be able to commit this change directly. There's some git info at https://wiki.php.net/vcs/gitworkflow and https://wiki.php.net/vcs/gitfaq, though it's a bit all over the place. Drop into R11 if you have any questions.

Thanks very much. I have committed this change to master.

@alexdowad alexdowad closed this Jun 17, 2020
@carusogabriel carusogabriel added this to the PHP 8.0 milestone Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants