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

Use local memory management #1859

Closed
wants to merge 9 commits into from

Conversation

@dreamsxin
Copy link
Member

commented May 5, 2019

See phalcon/cphalcon#14020

Generate code:

#define ZEPHIR_MM_GROW() \
	zephir_memory_def zephir_memory; \
	zephir_memory_grow_stack(&zephir_memory)

#define ZEPHIR_MM_RESTORE() zephir_memory_restore_stack(&zephir_memory)

PHP_METHOD(Test, test) {

	zend_long ZEPHIR_LAST_CALL_STATUS;
        // ....
	ZEPHIR_MM_GROW();
        // ....
	ZVAL_UNDEF(&x);
        // ....
	zephir_fetch_params(1, 1, 0, &x);
        // ....
	ZEPHIR_CALL_SELF(&ret, "xxx", NULL, 0, a, b);
	zephir_check_call_status();
        // ...
	ZEPHIR_MM_RESTORE();
}
@dreamsxin dreamsxin added the discussion label May 5, 2019
Library/ClassMethod.php Outdated Show resolved Hide resolved
@@ -137,7 +137,7 @@ public function genConcatCode()
$code .= "\t\t".'if (Z_TYPE_P(result) != IS_STRING) {'.PHP_EOL;
$code .= "\t\t\t".'use_copy = zend_make_printable_zval(result, &result_copy);'.PHP_EOL;
$code .= "\t\t\t".'if (use_copy) {'.PHP_EOL;
$code .= "\t\t\t\t".'ZEPHIR_CPY_WRT_CTOR(result, (&result_copy));'.PHP_EOL;

This comment has been minimized.

Copy link
@sergeyklay
@sergeyklay

This comment has been minimized.

Copy link
Member

commented May 8, 2019

@dreamsxin Looks really awesome

@sergeyklay

This comment has been minimized.

Copy link
Member

commented May 8, 2019

Could you remove ZendEngine2 tests from Travis CI ? I've remove this code a bit later

@dreamsxin

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

@sergeyklay You can remove it. There are many changes to the branch, especially those related to variables.

@@ -731,17 +1234,11 @@ public function copyOnWrite(Variable $target, $var, CompilationContext $context)
return;
}
return parent::copyOnWrite($target, $var, $context);
$context->codePrinter->output('ZEPHIR_CPY_WRT('.$this->getVariableCode($target).', '.$this->resolveValue($var, $context).');');

This comment has been minimized.

Copy link
@Nitpick-CI

Nitpick-CI May 9, 2019

Spaces must be used to indent lines; tabs are not allowed

$value = $value->getName();
} else {
$value = 'ZEPHIR_MM_ZVAL_STRING' == $macro ? '"'.$value.'"' : $value;
$doCopy = NULL;

This comment has been minimized.

Copy link
@Nitpick-CI

Nitpick-CI May 9, 2019

  • Spaces must be used to indent lines; tabs are not allowed
  • TRUE, FALSE and NULL must be lowercase; expected null but found NULL
@dreamsxin

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

@sergeyklay
Can you help me finish the next work and test?

Now error:
not ok 1 - Should correctly specify ARGINFO for ZendEngine3

@sergeyklay

This comment has been minimized.

Copy link
Member

commented May 9, 2019

@dreamsxin Yes, let me few days. I'm at vacation

@dreamsxin

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

@sergeyklay

Have a nice holiday.

@dreamsxin

This comment has been minimized.

Copy link
Member Author

commented May 15, 2019

@sjinks Hi, Can you help improve it?

@sergeyklay

This comment has been minimized.

Copy link
Member

commented May 17, 2019

@dreamsxin

$(phpenv which php) \
    -d extension=ext/modules/test.so \
    vendor/bin/simple-phpunit \
    --debug \
    --bootstrap unit-tests/ext-bootstrap.php \
    --testsuite Extension_Php72
Test 'Extension\AssignTest::shouldPerformAssignmentForProperties with data set #15 (array(true, false), 'testArrayBoolExpressionAssign')' started

php: /tmp/php-build/source/7.3.0/Zend/zend_hash.c:743: _zend_hash_str_add_or_update_i: Assertion `(zend_gc_refcount(&(ht)->gc) == 1) || ((ht)->u.flags & (1<<6))' failed.
Aborted (core dumped)

So, take a look at Extension\AssignTest::shouldPerformAssignmentForProperties

gdb --args \
    $(phpenv which php) -d extension=ext/modules/test.so \
    vendor/bin/simple-phpunit \
    --bootstrap unit-tests/ext-bootstrap.php \
    unit-tests/Extension/AssignTest.php
#0  0x00007ffff3d86428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff3d8802a in __GI_abort () at abort.c:89
#2  0x00007ffff3d7ebd7 in __assert_fail_base (fmt=<optimized out>, assertion=assertion@entry=0x1565248 "(zend_gc_refcount(&(ht)->gc) == 1) || ((ht)->u.flags & (1<<6))", file=file@entry=0x15651b0 "/tmp/php-build/source/7.3.0/Zend/zend_hash.c", line=line@entry=743, function=function@entry=0x1565670 <__PRETTY_FUNCTION__.11856> "_zend_hash_str_add_or_update_i") at assert.c:92
#3  0x00007ffff3d7ec82 in __GI___assert_fail (assertion=0x1565248 "(zend_gc_refcount(&(ht)->gc) == 1) || ((ht)->u.flags & (1<<6))", file=0x15651b0 "/tmp/php-build/source/7.3.0/Zend/zend_hash.c", line=743, function=0x1565670 <__PRETTY_FUNCTION__.11856> "_zend_hash_str_add_or_update_i") at assert.c:101
#4  0x0000000000c8b0e8 in _zend_hash_str_add_or_update_i (ht=0x7fffe3630540, str=0x7fffed5fcd18 "a", len=1, h=9223372036854953478, pData=0x7fffffffa6b0, flag=1) at /tmp/php-build/source/7.3.0/Zend/zend_hash.c:743
#5  0x0000000000c8b68a in zend_hash_str_update (ht=0x7fffe3630540, str=0x7fffed5fcd18 "a", len=1, pData=0x7fffffffa6b0) at /tmp/php-build/source/7.3.0/Zend/zend_hash.c:854
#6  0x00007fffe6e2f7d1 in zend_symtable_str_update (ht=0x7fffe3630540, str=0x7fffed5fcd18 "a", len=1, pData=0x7fffffffa6b0) at /home/klay/.phpenv/versions/7.3.0-zts-debug/include/php/Zend/zend_hash.h:498
#7  0x00007fffe6e3108e in zephir_update_property_array (object=0x7fffed42ac60, property=0x7fffe6fca4a6 "myArray", property_length=7, index=0x7fffffffa6a0, value=0x7fffffffa6b0) at /home/klay/work/zephir/php-zephir/ext/kernel/object.c:638
#8  0x00007fffe6e66621 in zim_Test_Assign_testArrayBoolExpressionAssign (execute_data=0x7fffed42ac40, return_value=0x7fffed42ab20) at /home/klay/work/zephir/php-zephir/ext/test/assign.zep.c:2108
#9  0x0000000000ce8b06 in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER () at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:1102
#10 0x0000000000d5b40e in execute_ex (ex=0x7fffed42aa00) at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:55442
#11 0x0000000000c5a7ce in zend_call_function (fci=0x7fffffffaae0, fci_cache=0x7fffffffaac0) at /tmp/php-build/source/7.3.0/Zend/zend_execute_API.c:756
#12 0x00000000008f8150 in reflection_method_invoke (execute_data=0x7fffed42a990, return_value=0x7fffed42a610, variadic=0) at /tmp/php-build/source/7.3.0/ext/reflection/php_reflection.c:3186
#13 0x00000000008f8302 in zim_reflection_method_invokeArgs (execute_data=0x7fffed42a990, return_value=0x7fffed42a610) at /tmp/php-build/source/7.3.0/ext/reflection/php_reflection.c:3222
#14 0x0000000000ce8b06 in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER () at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:1102
#15 0x0000000000d5b40e in execute_ex (ex=0x7fffed424030) at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:55442
#16 0x0000000000d60af0 in zend_execute (op_array=0x7fffed485300, return_value=0x0) at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:60834
#17 0x0000000000c7541d in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /tmp/php-build/source/7.3.0/Zend/zend.c:1568
#18 0x0000000000baea60 in php_execute_script (primary_file=0x7fffffffc380) at /tmp/php-build/source/7.3.0/main/main.c:2630
#19 0x0000000000d63b4a in do_cli (argc=7, argv=0x19c2000) at /tmp/php-build/source/7.3.0/sapi/cli/php_cli.c:997
#20 0x0000000000d64f95 in main (argc=7, argv=0x19c2000) at /tmp/php-build/source/7.3.0/sapi/cli/php_cli.c:1389
@sergeyklay

This comment has been minimized.

Copy link
Member

commented May 17, 2019

Also could you please accept this dreamsxin#1 and rebase

@sergeyklay

This comment has been minimized.

Copy link
Member

commented May 18, 2019

@dreamsxin

  1. Build test ext:
$ ZEPHIR_BIN=./zephir .ci/build-test-ext.sh
  1. Create test.php
<?php

use Test\Assign;

$test = new Assign();

$expected = [
    'a' => [
        'b_key' => 'b_val',
        'b' => ['d_key' => 'd_val', 'c' => ['d' => ['e' => 'f']]],
    ],
    1 => [2 => [3 => 4, 5 => 6, 'abc' => 'abc']],
    's' => 1,
];

assert($expected === $test->testPropertyArray14());
  1. Run test:
$ php -d extension=ext/modules/test.so test.php
php: /tmp/php-build/source/7.3.0/Zend/zend_hash.c:743: _zend_hash_str_add_or_update_i: Assertion `(zend_gc_refcount(&(ht)->gc) == 1) || ((ht)->u.flags & (1<<6))' failed.
Aborted (core dumped)
  1. Get backtrace:
gdb --args php -d extension=ext/modules/test.so test.php
#0  0x00007ffff3d86428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff3d8802a in __GI_abort () at abort.c:89
#2  0x00007ffff3d7ebd7 in __assert_fail_base (fmt=<optimized out>, assertion=assertion@entry=0x1565248 "(zend_gc_refcount(&(ht)->gc) == 1) || ((ht)->u.flags & (1<<6))", file=file@entry=0x15651b0 "/tmp/php-build/source/7.3.0/Zend/zend_hash.c", line=line@entry=743, function=function@entry=0x1565670 <__PRETTY_FUNCTION__.11856> "_zend_hash_str_add_or_update_i") at assert.c:92
#3  0x00007ffff3d7ec82 in __GI___assert_fail (assertion=0x1565248 "(zend_gc_refcount(&(ht)->gc) == 1) || ((ht)->u.flags & (1<<6))", file=0x15651b0 "/tmp/php-build/source/7.3.0/Zend/zend_hash.c", line=743, function=0x1565670 <__PRETTY_FUNCTION__.11856> "_zend_hash_str_add_or_update_i") at assert.c:101
#4  0x0000000000c8b0e8 in _zend_hash_str_add_or_update_i (ht=0x7fffed465a20, str=0x7fffe6fb0283 "b", len=1, h=9223372036854953479, pData=0x7fffffffa5a0, flag=1) at /tmp/php-build/source/7.3.0/Zend/zend_hash.c:743
#5  0x0000000000c8b68a in zend_hash_str_update (ht=0x7fffed465a20, str=0x7fffe6fb0283 "b", len=1, pData=0x7fffffffa5a0) at /tmp/php-build/source/7.3.0/Zend/zend_hash.c:854
#6  0x00007fffe6e270d2 in zephir_array_update_string (arr=0x7fffffffa590, index=0x7fffe6fb0283 "b", index_length=1, value=0x7fffffffa5a0, flags=0) at /home/klay/work/zephir/php-zephir/ext/kernel/array.c:784
#7  0x00007fffe6e27d32 in zephir_array_update_multi_ex (arr=0x7fffffffa950, value=0x7fffffffab60, types=0x7fffe6fb7632 "sss", types_length=3, types_count=6, ap=0x7fffffffa970) at /home/klay/work/zephir/php-zephir/ext/kernel/array.c:937
#8  0x00007fffe6e22018 in zephir_update_property_array_multi (object=0x7fffed4211a0, property=0x7fffe6fb7606 "myArray", property_length=7, value=0x7fffffffab60, types=0x7fffe6fb7632 "sss", types_length=3, types_count=6) at /home/klay/work/zephir/php-zephir/ext/kernel/object.c:722
#9  0x00007fffe6e51565 in zim_Test_Assign_testPropertyArray14 (execute_data=0x7fffed421180, return_value=0x7fffed4210e0) at /home/klay/work/zephir/php-zephir/ext/test/assign.zep.c:1597
#10 0x0000000000ce8b06 in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER () at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:1102
#11 0x0000000000d5b40e in execute_ex (ex=0x7fffed421030) at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:55442
#12 0x0000000000d60af0 in zend_execute (op_array=0x7fffed484300, return_value=0x0) at /tmp/php-build/source/7.3.0/Zend/zend_vm_execute.h:60834
#13 0x0000000000c7541d in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /tmp/php-build/source/7.3.0/Zend/zend.c:1568
#14 0x0000000000baea60 in php_execute_script (primary_file=0x7fffffffc400) at /tmp/php-build/source/7.3.0/main/main.c:2630
#15 0x0000000000d63b4a in do_cli (argc=4, argv=0x19c2000) at /tmp/php-build/source/7.3.0/sapi/cli/php_cli.c:997
#16 0x0000000000d64f95 in main (argc=4, argv=0x19c2000) at /tmp/php-build/source/7.3.0/sapi/cli/php_cli.c:1389

Full backtrace

@dreamsxin

This comment has been minimized.

Copy link
Member Author

commented May 19, 2019

I test use coroutine, The global fcall cache entry also needs to be removed.
.

dreamsxin added 6 commits May 20, 2019
dreamsxin added 3 commits May 22, 2019
@sergeyklay

This comment has been minimized.

Copy link
Member

commented May 25, 2019

@dreamsxin Could you please regenerate ext dir and rebase.

@sergeyklay

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

@dreamsxin I've merged some changes into development branch. Could you please regenerate ext dir and rebase.

@dreamsxin

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

@sergeyklay I close it, there are still many problems that can't be solved.

@dreamsxin dreamsxin closed this Jun 10, 2019
@dreamsxin dreamsxin referenced this pull request Jun 28, 2019
1 of 3 tasks complete
@sergeyklay sergeyklay changed the title [WIP]Use local memory management - DONT MERGE!! Use local memory management Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.