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
Closed

Use local memory management #1859

wants to merge 9 commits into from

Conversation

dreamsxin
Copy link
Contributor

@dreamsxin dreamsxin 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();
}

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sergeyklay
Copy link
Contributor

@dreamsxin Looks really awesome

@sergeyklay
Copy link
Contributor

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

@dreamsxin
Copy link
Contributor Author

@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).');');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@dreamsxin
Copy link
Contributor Author

dreamsxin 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
Copy link
Contributor

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

@dreamsxin
Copy link
Contributor Author

@sergeyklay

Have a nice holiday.

@dreamsxin
Copy link
Contributor Author

@sjinks Hi, Can you help improve it?

@sergeyklay
Copy link
Contributor

sergeyklay 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
Copy link
Contributor

sergeyklay commented May 17, 2019

Also could you please accept this dreamsxin#1 and rebase

@sergeyklay
Copy link
Contributor

@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
Copy link
Contributor Author

dreamsxin commented May 19, 2019

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

@sergeyklay
Copy link
Contributor

@dreamsxin Could you please regenerate ext dir and rebase.

@sergeyklay
Copy link
Contributor

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

@dreamsxin
Copy link
Contributor Author

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

@dreamsxin dreamsxin closed this Jun 10, 2019
@dreamsxin dreamsxin mentioned this pull request Jun 28, 2019
3 tasks
@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
Development

Successfully merging this pull request may close these issues.

3 participants