tarantool / tarantool-php

PECL PHP driver for Tarantool
http://tarantool.org/
Other
86 stars 24 forks source link

Add support php7.3 #148

Closed SonSergei closed 4 years ago

bigbes commented 5 years ago
SonSergei commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

What is the result?

stefansaraev commented 5 years ago

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

@SonSergei thanks much.

rybakit commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

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 commented 5 years ago

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 commented 4 years ago

@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_BENCHWORKERS=16; find reports -type f -not -name '*' -delete; cd benchmarks/Parallel && php ../../vendor/bin/phpbench run ../TarantoolBench.php --dump-file=../../reports/paralleltarantoolt16.xml --tag=paralleltarantoolt16 --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.

Totktonada commented 4 years ago

Superseded by PR #153.