Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [PATCH] use -prefer-non-pic on x86-{freebsd,linux} »

[PATCH] use -prefer-non-pic on x86-{freebsd,linux}

php.internals

Joe Orton

21 years ago
This is Rasmus' patch but only enabled on platforms where it's known to work. Applies to HEAD: the apache2handler SAPI builds and loads a shared extension OK like this on Linux/i686. It looks like some people preferred a configure flag for this and some preferred to do it by default. Consensus? Index: configure.in =================================================================== RCS file: /repository/php-src/configure.in,v retrieving revision 1.516 diff -u -r1.516 configure.in --- configure.in 21 Jul 2004 23:02:28 -0000 1.516 +++ configure.in 16 Sep 2004 09:26:39 -0000 @@ -1,4 +1,4 @@ -dnl ## $Id: configure.in,v 1.516 2004/07/21 23:02:28 edink Exp $ -*- sh -*- +dnl ## $Id: configure.in,v 1.516 2004/07/21 23:02:28 edink Exp $ -*- autoconf -*- dnl ## Process this file with autoconf to produce a configure script. divert(1) @@ -222,6 +222,19 @@ CPPFLAGS="$CPPFLAGS -D_XPG_IV";; esac +AC_MSG_CHECKING([whether to force non-PIC code in shared modules]) +# Disable PIC where it is known to be safe to do so, +# to avoid the performance hit from the lost register +case $host_alias in +i?86-*-linux*|i?86-*-freebsd*) + force_nonpic=yes + ;; +*) + force_nonpic=no + ;; +esac +AC_MSG_RESULT([$force_nonpic]) + dnl Include Zend and TSRM configurations. dnl ------------------------------------------------------------------------- @@ -860,8 +873,12 @@ fi ;; shared) + if test $force_nonpic = yes; then + standard_libtool_flag='-prefer-non-pic' + else + standard_libtool_flag='-prefer-pic' + fi enable_static=no - standard_libtool_flag=-prefer-pic EXTRA_LDFLAGS="$EXTRA_LDFLAGS -avoid-version -module" ;; esac Index: acinclude.m4 =================================================================== RCS file: /repository/php-src/acinclude.m4,v retrieving revision 1.273 diff -u -r1.273 acinclude.m4 --- acinclude.m4 12 Sep 2004 06:35:51 -0000 1.273 +++ acinclude.m4 16 Sep 2004 09:26:39 -0000 @@ -772,11 +772,16 @@ php_cxx_post=' && echo > $[@]' php_lo=o + if test $force_nonpic = yes; then + shared_c_meta='$(COMMON_FLAGS) $(CFLAGS_CLEAN) $(EXTRA_CFLAGS) -prefer-non-pic' + shared_cxx_meta='$(COMMON_FLAGS) $(CXXFLAGS_CLEAN) $(EXTRA_CXXFLAGS) -prefer-non-pic' + else + shared_c_meta='$(COMMON_FLAGS) $(CFLAGS_CLEAN) $(EXTRA_CFLAGS) -prefer-pic' + shared_cxx_meta='$(COMMON_FLAGS) $(CXXFLAGS_CLEAN) $(EXTRA_CXXFLAGS) -prefer-pic' + fi shared_c_pre='$(LIBTOOL) --mode=compile $(CC)' - shared_c_meta='$(COMMON_FLAGS) $(CFLAGS_CLEAN) $(EXTRA_CFLAGS) -prefer-pic' shared_c_post= shared_cxx_pre='$(LIBTOOL) --mode=compile $(CXX)' - shared_cxx_meta='$(COMMON_FLAGS) $(CXXFLAGS_CLEAN) $(EXTRA_CXXFLAGS) -prefer-pic' shared_cxx_post= shared_lo=lo @@ -1158,12 +1163,17 @@ install_modules="install-modules" PHP_MODULES="$PHP_MODULES \$(phplibdir)/$1.la" PHP_SUBST($2) + if test $force_nonpic = yes; then + php_shared_picflag="-prefer-non-pic" + else + php_shared_picflag="-prefer-pic" + fi cat >>Makefile.objects<<EOF \$(phplibdir)/$1.la: $3/$1.la \$(LIBTOOL) --mode=install cp $3/$1.la \$(phplibdir) $3/$1.la: \$($2) \$(translit($1,a-z_-,A-Z__)_SHARED_DEPENDENCIES) - \$(LIBTOOL) --mode=link ifelse($4,,[\$(CC)],[\$(CXX)]) \$(COMMON_FLAGS) \$(CFLAGS_CLEAN) \$(EXTRA_CFLAGS) \$(LDFLAGS) -o \[$]@ -export-dynamic -avoid-version -prefer-pic -module -rpath \$(phplibdir) \$(EXTRA_LDFLAGS) \$($2) \$(translit($1,a-z_-,A-Z__)_SHARED_LIBADD) + \$(LIBTOOL) --mode=link ifelse($4,,[\$(CC)],[\$(CXX)]) \$(COMMON_FLAGS) \$(CFLAGS_CLEAN) \$(EXTRA_CFLAGS) \$(LDFLAGS) -o \[$]@ -export-dynamic -avoid-version $php_shared_picflag -module -rpath \$(phplibdir) \$(EXTRA_LDFLAGS) \$($2) \$(translit($1,a-z_-,A-Z__)_SHARED_LIBADD) EOF ])

Sascha Schumann

21 years ago
On Thu, 16 Sep 2004, Joe Orton wrote:
> This is Rasmus' patch but only enabled on platforms where it's known to > work. Applies to HEAD: the apache2handler SAPI builds and loads a > shared extension OK like this on Linux/i686. It looks like some people > preferred a configure flag for this and some preferred to do it by > default. Consensus?
It needs to be optional. Otherwise, the DSO cannot be shared across multiple independent Apache instances (which is the main reason this has not been enabled for years). - Sascha

Joe Orton

21 years ago
On Thu, Sep 16, 2004 at 11:53:40AM +0200, Sascha Schumann wrote:
> On Thu, 16 Sep 2004, Joe Orton wrote: > >This is Rasmus' patch but only enabled on platforms where it's known to > >work. Applies to HEAD: the apache2handler SAPI builds and loads a > >shared extension OK like this on Linux/i686. It looks like some people > >preferred a configure flag for this and some preferred to do it by > >default. Consensus? > > It needs to be optional. Otherwise, the DSO cannot be shared > across multiple independent Apache instances (which is the > main reason this has not been enabled for years).
My understanding is that a single DSO on disk can be "shared" between two instances, but the text sections of such will not be shared in physical RAM between two instances which load it, because the code will get fixed up and hence copied-on-write. Is that not the case? So the trade-off for the default is between a performance hit in, I'd say, by far the the normal case (running a single Apache instance), or a memory hit in the less common case of running >1 instance concurrently. joe

Sascha Schumann

21 years ago
> My understanding is that a single DSO on disk can be "shared" between > two instances, but the text sections of such will not be shared in > physical RAM between two instances which load it, because the code will > get fixed up and hence copied-on-write. Is that not the case?
That is the correct understanding.
> So the trade-off for the default is between a performance hit in, I'd > say, by far the the normal case (running a single Apache instance), or a > memory hit in the less common case of running >1 instance concurrently.
The point is that the current behaviour needs to be retained for installations which would be negatively affected by your patch. Building non-PIC as default is ok, if a switch is provided to build PIC. - Sascha

Joe Orton

21 years ago
On Thu, Sep 16, 2004 at 12:39:39PM +0200, Sascha Schumann wrote:
> The point is that the current behaviour needs to be retained > for installations which would be negatively affected by your > patch. Building non-PIC as default is ok, if a switch is > provided to build PIC.
Ah, gotcha. Any preference as to flag name? --disable-non-pic would be an obvious choice, confusing double negative though. joe

Wez Furlong

21 years ago
Something like this sounds right: --enable-slow-pic Build a slower DSO that can be used to run multiple concurrent Apache instances. You don't need this unless you know why you need it. Which gets me thinking... how does this affect other SAPI (like CLI, embed) and extensions? --Wez. On Thu, 16 Sep 2004 11:46:04 +0100, Joe Orton <jorton@redhat.com> wrote:

Sascha Schumann

21 years ago
On Thu, 16 Sep 2004, Wez Furlong wrote:
> Something like this sounds right: > > --enable-slow-pic Build a slower DSO that can be used to run multiple > concurrent Apache instances. You don't > need this unless > you know why you need it. > > Which gets me thinking... how does this affect other SAPI (like CLI, > embed) and extensions?
CLI is a binary (no DSO, no PIC). Embed should probably be always built as PIC as it is supposed to be embedded as a library component. - Sascha

Wez Furlong

21 years ago
So we should enable pic automatically when the embed SAPI is selected; otherwise we are safe to go for non-pic, correct? --Wez. On Thu, 16 Sep 2004 14:07:42 +0200 (CEST), Sascha Schumann <sascha@schumann.cx> wrote:

Dmitry Stogov

21 years ago
All shared extensions (mysql,...) should be DSO, because they can be used by different Web Server modules and CLI PHP. Thanks. Dmitry.

Rasmus Lerdorf

21 years ago
On Thu, 16 Sep 2004, Dmitry Stogov wrote:
> All shared extensions (mysql,...) should be DSO, because they can be used by > different Web Server modules and CLI PHP.
This is still a memory vs. performance issue. In my case I definitely would prefer the performance, so I am not so sure that this holds here either. The common case is people using PHP in a single Apache instance and in a single SAPI. I think we should optimize for that. People who do more advanced things are by definition more advanced and as long as they have a knob to twiddle that will get them a true shared library if they want, they should be fine. I also don't think very many people have dozens of CLI php jobs running at the same time, so the extra couple of pages for that one-off CLI job really isn't going to hurt anybody. -Rasmus

Rasmus Lerdorf

21 years ago
On Thu, 16 Sep 2004, Joe Orton wrote:
> On Thu, Sep 16, 2004 at 12:39:39PM +0200, Sascha Schumann wrote: > > The point is that the current behaviour needs to be retained > > for installations which would be negatively affected by your > > patch. Building non-PIC as default is ok, if a switch is > > provided to build PIC. > > Ah, gotcha. Any preference as to flag name? --disable-non-pic would be > an obvious choice, confusing double negative though.
There is already a --without-pic/--with-pic flag that just doesn't work. I'd key it off of that. -Rasmus

Joe Orton

21 years ago
On Thu, Sep 16, 2004 at 06:31:56AM -0700, Rasmus Lerdorf wrote:
> On Thu, 16 Sep 2004, Joe Orton wrote: > > > On Thu, Sep 16, 2004 at 12:39:39PM +0200, Sascha Schumann wrote: > > > The point is that the current behaviour needs to be retained > > > for installations which would be negatively affected by your > > > patch. Building non-PIC as default is ok, if a switch is > > > provided to build PIC. > > > > Ah, gotcha. Any preference as to flag name? --disable-non-pic would be > > an obvious choice, confusing double negative though. > > There is already a --without-pic/--with-pic flag that just doesn't work. > I'd key it off of that.
It doesn't work because PHP prevents AC_PROG_LIBTOOL from interpreting it by doing "unset with_pic". So one solution would be to remove that hack and in the case where --with-pic or --without-pic is passed to configure, never explicitly specify a pic "preference" to the libtool script. And make the default as if "--without-pic" was passed in the x86* whitelist. Index: configure.in =================================================================== RCS file: /repository/php-src/configure.in,v retrieving revision 1.516 diff -u -r1.516 configure.in --- configure.in 21 Jul 2004 23:02:28 -0000 1.516 +++ configure.in 16 Sep 2004 14:07:03 -0000 @@ -1,4 +1,4 @@ -dnl ## $Id: configure.in,v 1.516 2004/07/21 23:02:28 edink Exp $ -*- sh -*- +dnl ## $Id: configure.in,v 1.516 2004/07/21 23:02:28 edink Exp $ -*- autoconf -*- dnl ## Process this file with autoconf to produce a configure script. divert(1) @@ -222,6 +222,21 @@ CPPFLAGS="$CPPFLAGS -D_XPG_IV";; esac +# Disable PIC mode by default where it is known to be safe to do so, +# to avoid the performance hit from the lost register +AC_MSG_CHECKING([whether to force non-PIC code in shared modules]) +case $host_alias in +i?86-*-linux*|i?86-*-freebsd*) + if test "${with_pic+set}" != "set"; then + with_pic=no + AC_MSG_RESULT(yes) + else + AC_MSG_RESULT(no) + fi + ;; +*) AC_MSG_RESULT(no) ;; +esac + dnl Include Zend and TSRM configurations. dnl ------------------------------------------------------------------------- @@ -850,7 +865,6 @@ enable_shared=yes enable_static=yes -unset with_pic case $php_build_target in program|static) @@ -861,7 +875,9 @@ ;; shared) enable_static=no - standard_libtool_flag=-prefer-pic + if test "${with_pic+set}" != set; then + standard_libtool_flag='-prefer-pic' + fi EXTRA_LDFLAGS="$EXTRA_LDFLAGS -avoid-version -module" ;; esac Index: acinclude.m4 =================================================================== RCS file: /repository/php-src/acinclude.m4,v retrieving revision 1.273 diff -u -r1.273 acinclude.m4 --- acinclude.m4 12 Sep 2004 06:35:51 -0000 1.273 +++ acinclude.m4 16 Sep 2004 14:07:03 -0000 @@ -772,11 +772,16 @@ php_cxx_post=' && echo > $[@]' php_lo=o + if test "${with_pic+set}" = set; then + shared_c_meta='$(COMMON_FLAGS) $(CFLAGS_CLEAN) $(EXTRA_CFLAGS)' + shared_cxx_meta='$(COMMON_FLAGS) $(CXXFLAGS_CLEAN) $(EXTRA_CXXFLAGS)' + else + shared_c_meta='$(COMMON_FLAGS) $(CFLAGS_CLEAN) $(EXTRA_CFLAGS) -prefer-pic' + shared_cxx_meta='$(COMMON_FLAGS) $(CXXFLAGS_CLEAN) $(EXTRA_CXXFLAGS) -prefer-pic' + fi shared_c_pre='$(LIBTOOL) --mode=compile $(CC)' - shared_c_meta='$(COMMON_FLAGS) $(CFLAGS_CLEAN) $(EXTRA_CFLAGS) -prefer-pic' shared_c_post= shared_cxx_pre='$(LIBTOOL) --mode=compile $(CXX)' - shared_cxx_meta='$(COMMON_FLAGS) $(CXXFLAGS_CLEAN) $(EXTRA_CXXFLAGS) -prefer-pic' shared_cxx_post= shared_lo=lo @@ -1158,12 +1163,15 @@ install_modules="install-modules" PHP_MODULES="$PHP_MODULES \$(phplibdir)/$1.la" PHP_SUBST($2) + if test ${with_pic+set} != set; then + php_shared_picflag="-prefer-pic" + fi cat >>Makefile.objects<<EOF \$(phplibdir)/$1.la: $3/$1.la \$(LIBTOOL) --mode=install cp $3/$1.la \$(phplibdir) $3/$1.la: \$($2) \$(translit($1,a-z_-,A-Z__)_SHARED_DEPENDENCIES) - \$(LIBTOOL) --mode=link ifelse($4,,[\$(CC)],[\$(CXX)]) \$(COMMON_FLAGS) \$(CFLAGS_CLEAN) \$(EXTRA_CFLAGS) \$(LDFLAGS) -o \[$]@ -export-dynamic -avoid-version -prefer-pic -module -rpath \$(phplibdir) \$(EXTRA_LDFLAGS) \$($2) \$(translit($1,a-z_-,A-Z__)_SHARED_LIBADD) + \$(LIBTOOL) --mode=link ifelse($4,,[\$(CC)],[\$(CXX)]) \$(COMMON_FLAGS) \$(CFLAGS_CLEAN) \$(EXTRA_CFLAGS) \$(LDFLAGS) -o \[$]@ -export-dynamic -avoid-version $php_shared_picflag -module -rpath \$(phplibdir) \$(EXTRA_LDFLAGS) \$($2) \$(translit($1,a-z_-,A-Z__)_SHARED_LIBADD) EOF ])

Rasmus Lerdorf

21 years ago
Makes sense to me. On Thu, 16 Sep 2004, Joe Orton wrote:

Derick Rethans

21 years ago
On Thu, 16 Sep 2004, Joe Orton wrote:
> On Thu, Sep 16, 2004 at 06:31:56AM -0700, Rasmus Lerdorf wrote: > > > > There is already a --without-pic/--with-pic flag that just doesn't work. > > I'd key it off of that. > > It doesn't work because PHP prevents AC_PROG_LIBTOOL from interpreting > it by doing "unset with_pic". > > So one solution would be to remove that hack and in the case where > --with-pic or --without-pic is passed to configure, never explicitly > specify a pic "preference" to the libtool script. And make the default > as if "--without-pic" was passed in the x86* whitelist. > > Index: configure.in > =================================================================== > RCS file: /repository/php-src/configure.in,v
<snip patch> Did we ever came to the conclusion whether we should put this patch into CVS. AFAIK everybody agreed with it... regards, Derick
-- Derick Rethans http://derickrethans.nl | http://ez.no | http://xdebug.org

George Schlossnagle

21 years ago
On Dec 7, 2004, at 10:45 AM, Derick Rethans wrote:
> > Did we ever came to the conclusion whether we should put this patch > into > CVS. AFAIK everybody agreed with it...
That's what I recall as well. George

Ilia A.

21 years ago
I believe that there was an all around agreement and this and given that we are nearing release of 5.0.3 and 4.3.10 this would be a perfect opportunity to do it. Otherwise it'll need wait for unknown number of months till the next release. Ilia