Skip to content
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

Add support php7.3 #148

Closed
wants to merge 4 commits into from
Closed

Add support php7.3 #148

wants to merge 4 commits into from

Conversation

SonSergei
Copy link

No description provided.

@SonSergei SonSergei mentioned this pull request Apr 3, 2019
@bigbes
Copy link
Contributor

bigbes commented Apr 4, 2019

  • Use tabs instead of spaces.
  • Travis CI - you can see that build matrix became very big
  • Are you sure ARRAY_PROTECT_RECURSION... exists in PHP code? "grep"ing source code shows me 0 results

@SonSergei
Copy link
Author

Use tabs instead of spaces. -> OK
Travis CI -> .travis.yml already broken
ARRAY_PROTECT_RECURSION -> its my macros of compatibility with php73, see src/php_tarantool.h

@bigbes
Copy link
Contributor

bigbes commented Apr 4, 2019

Travis CI -> .travis.yml already broken

Then don't modify it?

ARRAY_PROTECT_RECURSION -> its my macros of compatibility with php73, see src/php_tarantool.h. View changes

My bad, overlooked #else. Can you add two spaces after "#" and before "define", To show that it's inside #if .. #else .. #endif block?

Nevertheless LGTM

@SonSergei
Copy link
Author

What is the result?

@stefansaraev
Copy link
Contributor

@bigbes fyi, this runs fine for us (bm) on debian stretch + php7.0 and debian buster + php7.3

@SonSergei thanks much.

@rybakit
Copy link
Collaborator

rybakit commented Oct 19, 2019

I found that it doesn't work on PHP ZTS, tested on version 7.3.10 and got:

(/usr/lib64/php-zts/modules/tarantool.so: undefined symbol: compiler_globals)) in Unknown on line 0

@SonSergei
Copy link
Author

@rybakit I can not reproduce, can you help?
I used the following script:

docker run --rm -it centos:7 bash

## Instal php 7.3
yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm https://rpms.remirepo.net/enterprise/remi-release-7.rpm yum-utils && \
yum-config-manager --enable remi-php73 && \
yum install -y php-zts php-devel && \
php --version

## Instal php 7.3 ZTS
yum install -y git make bison && \
git clone --branch=PHP-7.3.10 --depth=1  https://github.com/php/php-src /usr/src/php-src && \
cd /usr/src/php-src && \
./buildconf --force && \
./configure --prefix=/opt/php-zts --with-config-file-path=/opt/php-zts/etc --enable-maintainer-zts  --disable-all && \
make && make install && \
/opt/php-zts/bin/php --version

## Instal tarantool php
yum install -y git make && \
git clone --branch=php7.3 --depth=1 https://github.com/SonSergei/tarantool-php.git /usr/src/php-tarantool
cd /usr/src/php-tarantool && \
/opt/php-zts/bin/phpize && ./configure --with-php-config=/opt/php-zts/bin/php-config && \
make && make install && \
/opt/php-zts/bin/php -dextension=tarantool.so -m && \
/opt/php-zts/bin/php -dextension=tarantool.so -r "echo class_exists('\Tarantool').PHP_EOL;"

@rybakit
Copy link
Collaborator

rybakit commented Oct 21, 2019

@SonSergei I've just rechecked it and it indeed works 👍 :

docker run --rm -it fedora:30 bash
dnf install php-devel php-zts git make
git clone --branch=php7.3 --depth=1 https://github.com/SonSergei/tarantool-php.git /usr/src/php-tarantool
cd /usr/src/php-tarantool
phpize && ./configure --with-php-config=/usr/bin/zts-php-config && make && make install

I think I forgot to pass zts-php-config to ./configure when I was installing it before.

@rybakit
Copy link
Collaborator

rybakit commented Oct 21, 2019

Another thing, now I get a zend_mm_heap corrupted error on ZTS on update and insert operations. I'm not sure if it's related to ext-tarantool or ext-parallel I'm using.

@SonSergei if you want to investigate it, clone this repo, install zts-php and ext-parallel and run

make clean bench-parallel-connectors

UPDATE: a full reproducer:

git clone --depth=1 https://github.com/tarantool-php/benchmarks
cd benchmarks
docker run -d --network host --name=tarantool-bench -v $PWD/bench.lua:/bench.lua tarantool/tarantool:2 tarantool /bench.lua
docker run --network host -v $PWD:/benchmarks --rm -it fedora:30 bash

dnf install -y php-devel php-zts php-opcache php-composer-installers git make which
git clone --branch=php7.3 --depth=1 https://github.com/SonSergei/tarantool-php.git /usr/src/php-tarantool
cd /usr/src/php-tarantool
phpize && ./configure --with-php-config=/usr/bin/zts-php-config && make && make install

git clone --depth=1 https://github.com/msgpack/msgpack-php.git /usr/src/ext-msgpack
cd /usr/src/ext-msgpack
phpize && ./configure --with-php-config=/usr/bin/zts-php-config && make && make install

git clone --depth=1 https://github.com/krakjoe/parallel.git /usr/src/ext-parallel
cd /usr/src/ext-parallel
phpize && ./configure --with-php-config=/usr/bin/zts-php-config && make && make install

cd /benchmarks
make clean bench-parallel-connectors

@Totktonada
Copy link
Member

@rybakit Thanks a lot for catching this!

I reproduced it on php 7.2 without the patch, so the problem is not the regression from the patch. Filed separate issue: #152.

I prepared the environment for PHP 7.2 ZTS (with thread safety) with msgpack.so, parallel.so and tarantool.so and run the following commands from tarantool-php/benchmarks directory:

$ composer install
$ (export TNT_BENCH_TARANTOOL_URI=tcp://localhost:3301; export TNT_BENCH_WORKERS=16; find reports -type f -not -name '_*' -delete; cd benchmarks/Parallel && php ../../vendor/bin/phpbench run ../TarantoolBench.php --dump-file=../../reports/parallel__tarantool__t16.xml --tag=parallel__tarantool__t16 --retry-threshold=3 --filter=update)

I tried to reproduce it with valgrind, but it is VERY slow (and one-threaded). No luck here for now.

This was referenced Mar 25, 2020
@Totktonada
Copy link
Member

Superseded by PR #153.

@Totktonada Totktonada closed this Mar 26, 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.

5 participants