scikit-hep/root_numpy

install from source does not have optimizations enabled

wlav opened this issue · 8 comments

wlav commented

See detailed comment on the joss review ...

I cloned the repository, then ran simply python setup.py install. This does NOT enable optimizations for the .cpp code. Net effect isn't too bad per se, b/c the reading dominates, but the overhead is not insignificant: quite a bit of code can be inlined and the generated cython code has an ugly but simply to resolve block structure. As such, optimization really does help ... You might want to enforce it.

ndawe commented

Thanks for pointing this out. Definitely, we should enable optimisation.

ndawe commented

When I build I see that -O2 is already used since that must have been enabled in the build of python on my machine (the same flags are borrowed in distutils). Is -O2 not there by default for you?

wlav commented

Yes, -O2 is there for the python build (although on e.g. my Mac laptop it's not, that only has -Os). But it's definitely not picking CFLAGS up from distutils (which is: "'-fno-strict-aliasing -O2 -pipe -march=native -fwrapv -DNDEBUG'"), as clearly none of its arguments are seen:

x86_64-pc-linux-gnu-g++ -pthread -fPIC -Iroot_numpy/src -I/usr/include/python2.7 -I/home/wlav/noel/rnp/lib/python2.7/site-packages/numpy/core/include -c root_numpy/src/_librootnumpy.cpp -o build/temp.linux-x86_64-2.7/root_numpy/src/_librootnumpy.o -pthread -std=c++11 -Wno-deprecated-declarations -m64 -I/home/wlav/aditi/build/include -Wno-unused-function -Wno-write-strings

ndawe commented

Strange. I suppose we can detect if it's missing and add it in that case. This is how root_numpy builds for me:

python setup.py build_ext -i
running build_ext
building 'root_numpy._librootnumpy' extension
creating build
creating build/temp.linux-x86_64-2.7
creating build/temp.linux-x86_64-2.7/root_numpy
creating build/temp.linux-x86_64-2.7/root_numpy/src
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-ZZaKJ6/python2.7-2.7.13=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -Iroot_numpy/src -I/usr/include/python2.7 -I/home/endw/.local/lib/python2.7/site-packages/numpy/core/include -c root_numpy/src/_librootnumpy.cpp -o build/temp.linux-x86_64-2.7/root_numpy/src/_librootnumpy.o -pthread -std=c++11 -m64 -I/home/endw/software/ROOT/current/include -Wno-unused-function -Wno-write-strings
ndawe commented

@wlav did you check the CFLAGS with the following?

python -c "import sysconfig; print sysconfig.get_config_var('CFLAGS')"
-fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-ZZaKJ6/python2.7-2.7.13=. -fstack-protector-strong -Wformat -Werror=format-security
ndawe commented

and do you have setuptools installed?

easy_install --version
setuptools 36.2.7 from /home/endw/.local/lib/python2.7/site-packages (Python 2.7)
wlav commented

I checked the CFLAGS through distutils.sysconfig, but I presume in the end it's all the same. Either way:

$ python -c "import sysconfig; print sysconfig.get_config_var('CFLAGS')"
-fno-strict-aliasing -O2 -pipe -march=native -fwrapv -DNDEBUG
$ easy_install --version`
setuptools 36.2.7 from /home/wlav/noel/rnp/lib/python2.7/site-packages (Python 2.7)

so nothing strange there per se.

I tried on an OpenSuSE box and an SLC6 one. The difference is apparently the OPT config_var, which has no optimization flags on the gentoo box, but a whole slew (both -O2 and -O3 among them) on OpenSuSE and SLC6. But then the gentoo one still has -DNDEBUG in OPT, so there must be more ...

Indeed, I see on my OpenSuSE box in distutils.sysconfig:

        if 'CFLAGS' in os.environ:
            cflags = opt + ' ' + os.environ['CFLAGS']

whereas the gentoo box has:

        if 'CFLAGS' in os.environ:
            cflags = os.environ['CFLAGS']

One difference is that the gentoo box is 2.7.12, the OpenSuSE one is 2.7.9, so I checked the source, but no: Python from python.org (both .12 and the more recent .13 that you have) has the opt added into the mix.

Then I bing'ed it and came across this (for pypy distutils, not cpython, but clearly gentoo messes with it):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=8bb3a46e04cc4583c4f115506c711e20e5166ca7

Well, well. One more reason to hate gentoo. Aargh!

eduardo-rodrigues commented

I believe it is fair to close this task at this point.