[PHP-DEV] Urgent patch for traits in PHP 5.4
Hi,
Recently we've found a huge problem in PHP traits implementation.
It performs copying of each op_array (with all opcodes!) for each method
used from trait. This not only makes traits extremely slow and reduce
effect of opcode caches, but also prohibits extensions from modifying
op_array in some way, e.g. extending op_arrays with additional
information in op_array.reserved* fields. As result some extensions
(e.g. xdebug and some Zend extensions) will just crash on traits usage.
As I understood the copying was done only for proper handling of
__CLASS__ constant in trait methods. I think it's too radical solution.
I've introduced ZEND_CLASS_NAME instruction instead and made op_arrays
to share their opcodes (in the same way as inherited methods). The only
difference that methods from traits may be renamed.
The patch is attached (it requires executor/scanner/parser regeneration)
I would like to commit it into 5.4 and trunk. Note that the patch makes
binary compatibility break and can't be applied to 5.4.* after 5.4.0
release.
I know that applying it may delay the PHP 5.4.0 release, but it's better
to fix the problem now.
Thanks. Dmitry.
Index: Zend/zend_compile.c
===
--- Zend/zend_compile.c (revision 322174)
+++ Zend/zend_compile.c (working copy)
@@ -2226,6 +2226,19 @@
}
/* }}} */
+void zend_do_class_name(znode *result TSRMLS_DC) /* {{{ */
+{
+ zend_op *opline = get_next_op(CG(active_op_array) TSRMLS_CC);
+
+ opline-opcode = ZEND_CLASS_NAME;
+ SET_UNUSED(opline-op1);
+ SET_UNUSED(opline-op2);
+ opline-result_type = IS_TMP_VAR;
+ opline-result.var = get_temporary_variable(CG(active_op_array));
+ GET_NODE(result, opline-result);
+}
+/* }}} */
+
void zend_do_label(znode *label TSRMLS_DC) /* {{{ */
{
zend_label dest;
@@ -3670,142 +3683,6 @@
}
/* }}} */
-/* {{{ Originates from php_runkit_function_copy_ctor
- Duplicate structures in an op_array where necessary to make an outright
duplicate */
-static void zend_traits_duplicate_function(zend_function *fe, zend_class_entry
*target_ce, char *newname TSRMLS_DC)
-{
- zend_literal *literals_copy;
- zend_compiled_variable *dupvars;
- zend_op *opcode_copy;
- zval class_name_zv;
- int class_name_literal;
- int i;
- int number_of_literals;
-
- if (fe-op_array.static_variables) {
- HashTable *tmpHash;
-
- ALLOC_HASHTABLE(tmpHash);
- zend_hash_init(tmpHash,
zend_hash_num_elements(fe-op_array.static_variables), NULL, ZVAL_PTR_DTOR, 0);
- zend_hash_apply_with_arguments(fe-op_array.static_variables
TSRMLS_CC, (apply_func_args_t)zval_copy_static_var, 1, tmpHash);
-
- fe-op_array.static_variables = tmpHash;
- }
-
- number_of_literals = fe-op_array.last_literal;
- literals_copy = (zend_literal*)emalloc(number_of_literals *
sizeof(zend_literal));
-
- for (i = 0; i number_of_literals; i++) {
- literals_copy[i] = fe-op_array.literals[i];
- zval_copy_ctor(literals_copy[i].constant);
- }
- fe-op_array.literals = literals_copy;
-
-
- fe-op_array.refcount = emalloc(sizeof(zend_uint));
- *(fe-op_array.refcount) = 1;
-
- if (fe-op_array.vars) {
- i = fe-op_array.last_var;
- dupvars = safe_emalloc(fe-op_array.last_var,
sizeof(zend_compiled_variable), 0);
- while (i 0) {
- i--;
- dupvars[i].name = estrndup(fe-op_array.vars[i].name,
fe-op_array.vars[i].name_len);
- dupvars[i].name_len = fe-op_array.vars[i].name_len;
- dupvars[i].hash_value = fe-op_array.vars[i].hash_value;
- }
- fe-op_array.vars = dupvars;
- } else {
- fe-op_array.vars = NULL;
- }
-
- opcode_copy = safe_emalloc(sizeof(zend_op), fe-op_array.last, 0);
- for(i = 0; i fe-op_array.last; i++) {
- opcode_copy[i] = fe-op_array.opcodes[i];
- if (opcode_copy[i].op1_type != IS_CONST) {
- switch (opcode_copy[i].opcode) {
- case ZEND_GOTO:
- case ZEND_JMP:
- if (opcode_copy[i].op1.jmp_addr
opcode_copy[i].op1.jmp_addr = fe-op_array.opcodes
- opcode_copy[i].op1.jmp_addr
fe-op_array.opcodes + fe-op_array.last) {
- opcode_copy[i].op1.jmp_addr =
opcode_copy + (fe-op_array.opcodes[i].op1.jmp_addr - fe-op_array.opcodes);
- }
- break;
- }
- } else {
- /* if __CLASS__ i.e. T_CLASS_C was used, we need to fix
it up here */
-
Re: [PHP-DEV] Urgent patch for traits in PHP 5.4
Hi Stefan, On 01/13/2012 02:13 PM, Stefan Marr wrote: Hi Dmitry: On 13 Jan 2012, at 10:36, Dmitry Stogov wrote: Recently we've found a huge problem in PHP traits implementation. Thanks for taking care of it, but just to be explicit here: I pointed out the implementation details in the various discussions. I never made it 'a secret' that there is copying going on. I even tried to point out the potential for this kind of sharing. See for instance: http://markmail.org/message/okhq2vp7h3yuegot And the comment of the initial commit: http://svn.php.net/viewvc?view=revisionrevision=298348 Sorry, but I am a bit annoyed that 'the community' does not care enough about reviewing such engine changes. I got plenty of help from various people, but 'discovering such a huge problem' so late in the process points out certain problems. Sorry, I didn't look into traits implementation before. Just was pointed into the problem. Anyway, back to the technical details. It performs copying of each op_array (with all opcodes!) for each method used from trait. This not only makes traits extremely slow and reduce effect of opcode caches, but also prohibits extensions from modifying op_array in some way, e.g. extending op_arrays with additional information in op_array.reserved* fields. As result some extensions (e.g. xdebug and some Zend extensions) will just crash on traits usage. As I understood the copying was done only for proper handling of __CLASS__ constant in trait methods. I think it's too radical solution. I've introduced ZEND_CLASS_NAME instruction instead and made op_arrays to share their opcodes (in the same way as inherited methods). The only difference that methods from traits may be renamed. From the top of my head, it is the handling of __CLASS__ and the handling of static variables in methods. You did not mention that, is it taken care of explicitly, or do traits now share static state? The later would not be intended. Will check whether we got that documented with a test. static variables are separated in the same way as for inherited methods (it works out of the box). The patch is attached (it requires executor/scanner/parser regeneration) I would like to commit it into 5.4 and trunk. Note that the patch makes binary compatibility break and can't be applied to 5.4.* after 5.4.0 release. I know that applying it may delay the PHP 5.4.0 release, but it's better to fix the problem now. I would be in favor of getting it into the process now, too. Especially if it breaks binary compatibility. I will take at a look at the patch later today. Yes, please do. All tests are passed, but I may miss something. Thanks. Dmitry. Thanks Stefan -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: SOAP
Hi Alex,
Right now it's possible to use SOAP with proxy using special options in
SoapClient constructor
(http://www.php.net/manual/en/soapclient.soapclient.php)
new SoapClient($wsdl, array('proxy_host' = 'my.proxy.com',
'proxy_port' = '8080',
'proxy_login' = '',
'proxy_password' = ''));
But I understood, that you like to use default proxy values to avoid
changes in third-party application. So I think the patch makes sense.
Stas, Ilia, any objections against committing it into 5.4 and 5.3?
Thanks. Dmitry.
On 01/10/2012 12:31 AM, Alex Samorukov wrote:
Hi Dmitry,
I created a small enchantment for the SOAP extension, could you please
take a look?
https://bugs.php.net/bug.php?id=60676
We have some clients in proxy enabled environments and its possible to
force proxy usage in curl or php streams, but not in SOAP. Proposed
patch adds new ini settings to resolve this. Please, tell if i need to
change anything in it.
Thank you.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Zend Multibyte support
Hi Yasuo, how did you see that Zend Multibyte Support weren't enabled? $ sapi/cli/php -d zend.multibyte=1 -i | grep -i multibyte Zend Multibyte Support = provided by mbstring zend.multibyte = On = On ** Multibyte Support = enabled Multibyte string engine = libmbfl Multibyte (japanese) regex support = enabled Multibyte regex (oniguruma) backtrack check = On Multibyte regex (oniguruma) version = 4.7.1 Thanks. Dmitry. On 11/03/2011 04:32 AM, Yasuo Ohgaki wrote: Hi all, I noticed that Zend Multibyte Support won't be on with ./sapi/cli/php -d zend.multibyte=1 nor zend.multibyte=on (in php.ini) This happens both php-src and php-src-5.4. According to php.ini-production from php-src: ; If enabled, scripts may be written in encodings that are incompatible with ; the scanner. CP936, Big5, CP949 and Shift_JIS are the examples of such ; encodings. To use this feature, mbstring extension must be enabled. ; Default: Off ;zend.multibyte = Off I thought it became runtime option. Is this a bug or am I missing something? -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Zend Multibyte support
I suppose PHP can't autodetect SJIS encoding and needs a hint for 5.4 $ php -d zend.multibyte=1 -d zend.script_encoding=SJIS sjis.php 5.3 must be compiled with --enable-zend-multibyte, then $ php -d mbstring.script_encoding=SJIS for 5.3 (I've just tested 5.4 but not 5.3. Just don't have 5.3 compiled with --enable-zend-multibyte). Thanks. Dmitry. On 11/03/2011 12:12 PM, Yasuo Ohgaki wrote: Hi Dimity Rui, I was trying to see if PHP 5.4 and trunk was also affected by this bug report. --with-zend-multibyte and --enable-debug reports LEAK with run-test.php https://bugs.php.net/bug.php?id=60194 So I configured as ./configure --enable-debug Sorry for being a lazy reader. I could turn on zend.multibyte with --enable-mbstring. Now I see PHP 5.3 doesn't have dependency to mbstring, but PHP 5.4+ has. I haven't read the last sentence.Thanks. It looks like php-src-5.4 doesn't has leaks as PHP 5.3 does. (I haven't completed the test, since it seems zend.multibyte is NOT working) $ TEST_PHP_EXECUTABLE=./sapi/cli/php ./run-tests.php -m -c ./php.ini = PHP : ./sapi/cli/php PHP_SAPI: cli PHP_VERSION : 5.4.0RC1-dev ZEND_VERSION: 2.4.0 PHP_OS : Linux - Linux dev.inter.es-i.jp 2.6.35.14-2m.mo7.x86_64 #1 SMP Mon Sep 12 11:09:50 JST 2011 x86_64 INI actual : /home/yohgaki/ext/svn/oss/php.net/php-src-5.4/php.ini More .INIs : CWD : /home/yohgaki/ext/svn/oss/php.net/php-src-5.4 Extra dirs : VALGRIND: valgrind-3.6.1 = TIME START 2011-11-03 16:36:57 = PASS EXPECT [tests/run-test/test001.phpt] PASS EXPECTF [tests/run-test/test002.phpt] PASS EXPECTREGEX [tests/run-test/test003.phpt] . However, I got the same parse error, as Rui mentioned, with SJIS source script. sjis.php ?php echo '表'; (You cannot copypaste, since this would be UTF-8. If you need file, let me know. I'll directly mail you.) 表 is valid SJIS char code and has \ as second byte. Since PHP complains with parse error, it seems zend.multibyte is not working on PHP 5.4. $ ./sapi/cli/php -i | grep multibyte zend.multibyte = On = On $ ./sapi/cli/php /usr/local/apache2.0/htdocs/sjis.php PHP Parse error: syntax error, unexpected ''�\';' (T_ENCAPSED_AND_WHITESPACE) in /usr/local/apache2.0/htdocs/sjis.php on line 2 It seems like working with zend.multibyte=Off. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net On Thu, Nov 3, 2011 at 4:09 PM, Dmitry Stogovdmi...@zend.com wrote: Hi Yasuo, how did you see that Zend Multibyte Support weren't enabled? $ sapi/cli/php -d zend.multibyte=1 -i | grep -i multibyte Zend Multibyte Support = provided by mbstring zend.multibyte = On = On ** Multibyte Support = enabled Multibyte string engine = libmbfl Multibyte (japanese) regex support = enabled Multibyte regex (oniguruma) backtrack check = On Multibyte regex (oniguruma) version = 4.7.1 Thanks. Dmitry. On 11/03/2011 04:32 AM, Yasuo Ohgaki wrote: Hi all, I noticed that Zend Multibyte Support won't be on with ./sapi/cli/php -d zend.multibyte=1 nor zend.multibyte=on (in php.ini) This happens both php-src and php-src-5.4. According to php.ini-production from php-src: ; If enabled, scripts may be written in encodings that are incompatible with ; the scanner. CP936, Big5, CP949 and Shift_JIS are the examples of such ; encodings. To use this feature, mbstring extension must be enabled. ; Default: Off ;zend.multibyte = Off I thought it became runtime option. Is this a bug or am I missing something? -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Zend Multibyte support
$ sapi/cli/php -d zend.multibyte=1 -d zend.script_encodinSJIS -d mbstring.internal_encoding=UTF8 -d mbstring.output_encoding=UTF8 sjis.php 表 Too many different encodings :) Thanks. Dmitry. On 11/03/2011 01:38 PM, Yasuo Ohgaki wrote: Oops, I thought ? was due to terminal encoding, but I double checked with redirecting to a file. $ ./sapi/cli/php -d zend.multibyte=1 -d zend.script_encoding=SJIS sjis.php tt It became ? instead of 表.. It seems something wrong. Thanks for you time. -- Yasuo Ohgaki yohg...@ohgaki.net On Thu, Nov 3, 2011 at 6:27 PM, Yasuo Ohgakiyohg...@ohgaki.net wrote: Hi Dimity, Now it seems working as it supposed. Thanks. $ ./sapi/cli/php -d zend.multibyte=1 -d zend.script_encoding=SJIS sjis.php? (? is due to my terminal encoding. It sets to UTF-8) It seems LEAK problem was gone with PHP 5.4, too. [yohgaki@dev php-src-5.4]$ TEST_PHP_EXECUTABLE=./sapi/cli/php ./run-tests.php -m -c ./php.ini = PHP : ./sapi/cli/php PHP_SAPI: cli PHP_VERSION : 5.4.0RC1-dev ZEND_VERSION: 2.4.0 PHP_OS : Linux - Linux dev.inter.es-i.jp 2.6.35.14-2m.mo7.x86_64 #1 SMP Mon Sep 12 11:09:50 JST 2011 x86_64 INI actual : /home/yohgaki/ext/svn/oss/php.net/php-src-5.4/php.ini More .INIs : CWD : /home/yohgaki/ext/svn/oss/php.net/php-src-5.4 Extra dirs : VALGRIND: valgrind-3.6.1 = TIME START 2011-11-03 18:23:53 = PASS EXPECT [tests/run-test/test001.phpt] PASS EXPECTF [tests/run-test/test002.phpt] PASS EXPECTREGEX [tests/run-test/test003.phpt] PASS INI section allows '=' [tests/run-test/test004.phpt] PASS Error message handling (without ZendOptimizer) [tests/run-test/test005.phpt] PASS Error messages are shown [tests/run-test/test006.phpt] PASS dirname test [tests/run-test/test007.phpt] -- Yasuo Ohgaki yohg...@ohgaki.net On Thu, Nov 3, 2011 at 6:16 PM, Dmitry Stogovdmi...@zend.com wrote: php -d zend.multibyte=1 -d zend.script_encoding=SJIS sjis.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Garbage collector and objects
Hi Gustavo, First of all, thank you for catching this. For now, I've tried to extend GC to support closures. You can find the proposed patch at https://bugs.php.net/bug.php?id=60139 However it won't work for other classes, anyway. Probably it would be better to extend object handlers in 5.4 with some uniform GC related callback, but for now I don't see how it should look like. Any ideas are welcome. Thanks. Dmitry. On 10/27/2011 08:22 PM, Gustavo Lopes wrote: The garbage_collector currently only looks for relationships of objects by calling get_properties. This causes noncollectable cycles in many internal classes and several bug reports (the latest of which is https://bugs.php.net/bug.php?id=60139 ). The only solution I found so far is very very hacky and I'm not completely convinced it works properly: http://lxr.php.net/opengrok/xref/PHP_TRUNK/ext/spl/spl_observer.c#377 Do you think it would be possible for you to extend the current object model and possibly the garbage collector to handle these situations before 5.4 (or give me some pointers)? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Garbage collector and objects
I've also wrote another patch (gc-closure2.diff at https://bugs.php.net/bug.php?id=60139) that introduce new get_gc() object handler in 5.4. Thanks. Dmitry. On 10/31/2011 12:51 PM, Dmitry Stogov wrote: Hi Gustavo, First of all, thank you for catching this. For now, I've tried to extend GC to support closures. You can find the proposed patch at https://bugs.php.net/bug.php?id=60139 However it won't work for other classes, anyway. Probably it would be better to extend object handlers in 5.4 with some uniform GC related callback, but for now I don't see how it should look like. Any ideas are welcome. Thanks. Dmitry. On 10/27/2011 08:22 PM, Gustavo Lopes wrote: The garbage_collector currently only looks for relationships of objects by calling get_properties. This causes noncollectable cycles in many internal classes and several bug reports (the latest of which is https://bugs.php.net/bug.php?id=60139 ). The only solution I found so far is very very hacky and I'm not completely convinced it works properly: http://lxr.php.net/opengrok/xref/PHP_TRUNK/ext/spl/spl_observer.c#377 Do you think it would be possible for you to extend the current object model and possibly the garbage collector to handle these situations before 5.4 (or give me some pointers)? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Ternary operator performance improvements
Hi,
As I already said, I don't see any technical issues with the patch.
I don't like it very mach, but it looks robust, and I don't object about it.
Thanks. Dmitry.
On 10/14/2011 10:08 PM, Arnaud Le Blanc wrote:
Hi,
I've already posted this patch and it has since been reviewed and improved.
I'm re-posting it for discussion before eventually commiting it.
The ternary operator always copies its second or third operand, which is very
slow compared to an if/else when the operand is an array for example:
$a = range(0,9);
// this takes 0.3 seconds here:
for ($i = 0; $i 500; ++$i) {
if (true) {
$b = $a;
} else {
$b = $a;
}
}
// this takes 3.8 seconds:
for ($i = 0; $i 500; ++$i) {
$b = true ? $a : $a;
}
I've tried to reduce the performance hit by avoiding the copy when possible
(patch attached).
Benchmark:
Without patch: (the numbers are the time taken to run the code a certain
amount of times)
$int = 0;
$ary = array(1,2,3,4,5,6,7,8,9);
true ? 1 : 00.124
true ? 1+0 : 0 0.109
true ? $ary : 0 2.020 !
true ? $int : 0 0.103
true ? ${'ary'} : 0 2.290 !
true ?: 0 0.091
1+0 ?: 00.086
$ary ?: 0 2.151 !
${'var'} ?: 0 2.317 !
With patch:
true ? 1 : 00.124
true ? 1+0 : 0 0.195
true ? $ary : 0 0.103
true ? $int : 0 0.089
true ? ${'ary'} : 0 0.103
true ?: 0 0.086
1+0 ?: 00.159
$cv ?: 00.090
${'var'} ?: 0 0.089
The array copying overhead is eliminated. There is however a slowdown in some
of the cases, but overall there is no completely unexpected performance hit as
it is the case currently.
What do you think ? Is there any objection ?
Best regards,
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] ZE2 broken by newer gcc
Hi Flavius,
Unfortunately, the proposed fix is wrong.
It changes the operators precedence and it's definitely wrong.
I suppose the crash caused by side effect of some other bug.
Try to run the same script with valgrind.
Thanks. Dmitry.
On 09/11/2011 02:27 PM, Flavius Aspra wrote:
Hi
I think I've found a bug in the engine, and I think it occures only with
the latest gcc (gcc version 4.6.1 20110819 (prerelease)), since it
used to work with earlier versions.
For example line 867
http://lxr.php.net/opengrok/xref/PHP_5_3/Zend/zend_execute_API.c#867
should be
PZVAL_IS_REF((*fci-params)[i])) {
that's what has worked for me at least. Otherwise it ends with a segfault.
It may also be gcc breaking BC. So I'm not sure whose fault it is, but
I think [] takes precedence over * in the C specification.
Could someone please verify this issue?
Regards,
Flavius
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/NEWS branches/PHP_5_3/Zend/tests/bug53727.phpt branches/PHP_5_3/Zend/tests/is_a.phpt branches/PHP_5_3/Zend/zend_builtin_functions.c bran
Hi Ralph, I don't have strong opinion about your suggestion. In some cases this warning might help to find a typo, in others it may be useless, but anyway, I think that usage of undefined class name is a quite rare situation. I would prefer to keep it as is. Thanks. Dmitry. On 07/08/2011 06:55 PM, Ralph Schindler wrote: Oh by all means, kill the warning. The behavior is sound though (the fix for the issue reported in #53727 where a missing parent *ce would affect the is_a_impl()'s ability to correctly determine it's subtype. The question is, should we keep the warning in 5.4? I'd say no, I think false should suffice b/c it answers the question correctly: Is NonExistentClass a subclass of SomeOtherClass: no. In that case, there's no real need for a warning. Patch for 5.3 attached, and should probably be applied to trunk and 5.4 as well if you think that is the right idea. -ralph On 7/7/11 4:08 PM, Johannes Schlüter wrote: On Thu, 2011-07-07 at 20:27 +0200, Pierre Joye wrote: what kind of troubles do they see? https://pear.php.net/bugs/bug.php?id=18656 Even if the Warning is good it's questionable for 5.3, especially during RC. (note that warnings not only cause the error being reported, which can be switched off and fill logfiles but also triggers error handlers which won't expect this ...) johannes 2011/7/7 Johannes Schlüterjohan...@schlueters.de: Hi, I was told (didn't verify) that this causes lots of trouble with PEAR and other applications. We're in final RC phase of 5.3.7. I don't think it is critical for 5.3.7 and I wonder whether we need it for 5.3 at all. johannes On Mon, 2011-07-04 at 14:55 +, Dmitry Stogov wrote: dmitry Mon, 04 Jul 2011 14:55:39 + Revision: http://svn.php.net/viewvc?view=revisionrevision=312904 Log: Fixed bug #53727 (Inconsistent behavior of is_subclass_of with interfaces) Bug: https://bugs.php.net/53727 (Assigned) Inconsistent behavior of is_subclass_of with interfaces Changed paths: U php/php-src/branches/PHP_5_3/NEWS A php/php-src/branches/PHP_5_3/Zend/tests/bug53727.phpt U php/php-src/branches/PHP_5_3/Zend/tests/is_a.phpt U php/php-src/branches/PHP_5_3/Zend/zend_builtin_functions.c U php/php-src/branches/PHP_5_3/ext/standard/tests/class_object/is_a_variation_001.phpt A php/php-src/branches/PHP_5_4/Zend/tests/bug53727.phpt U php/php-src/branches/PHP_5_4/Zend/tests/is_a.phpt U php/php-src/branches/PHP_5_4/Zend/zend_builtin_functions.c U php/php-src/branches/PHP_5_4/ext/standard/tests/class_object/is_a_variation_001.phpt A php/php-src/trunk/Zend/tests/bug53727.phpt U php/php-src/trunk/Zend/tests/is_a.phpt U php/php-src/trunk/Zend/zend_builtin_functions.c U php/php-src/trunk/ext/standard/tests/class_object/is_a_variation_001.phpt -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Ternary operator improvements
Hi Arnaud,
Is the main advantage of the patch - elimination of zval copy
constructor for IS_VAR and IS_CV operands?
Does it improve speed only for VAR/CV array operands?
Will the following script require an extra zval ealloc/efree with the patch?
?php
function foo($x, $y) {
$x ? $y : default;
}
foo(false, something);
?
For now the patch still looks a bit messing to me. Also, I would
probably prefer to use new opcodes ZEND_QM_ASSIGN_VAR and ZEND_JMP_SET_VAR.
I'll try to think a bit more about it later today.
Thanks. Dmitry.
On 07/08/2011 06:39 PM, Arnaud Le Blanc wrote:
Hi,
Thanks for your answer.
Probably it would be better to support both TMP/VAR result. Or even use
different opcodes.
Yes. I've changed the patch so that it still uses IS_TMP_VAR when both
operands are IS_TMP_VAR or IS_CONST.
I've also added some benchmarks in micro_benchmark.php, and a test.phpt that
tests all combinations of (true|false) ? (const|tmp|var|cv) : (const|tmp|var|
cv) (and also for the ?: operator).
The test found a memory leak in the current trunk in the following code:
true ? $a : $b; the result of $b is marked UNUSED, but not the result of $a.
I've modified zend_do_free() to add a ZEND_FREE op after the whole expression
instead.
Best regards,
Arnaud
Le Friday 8 July 2011 07:51:22, vous avez écrit :
Hi Arnaud,
I haven't test your patch yet, but I got the main idea.
I'll need to carefully analyze it. Probably it would be better to
support both TMP/VAR result. Or even use different opcodes.
Thanks. Dmitry.
On 07/07/2011 10:45 PM, Arnaud Le Blanc wrote:
Hi Dmitry,
The ternary operator always deep-copies its second or third operand, and
it is very slow compared to an if/else when the operand is an array for
example.
I tried to improve this by avoiding copies when possible. I ran the test
suite and seen no problem, but I'm not sure if this is correct or not;
could you please review the attached patch ?
The following script shows the improvements:
$a = range(1, 1);
for($i = 0; $i 10; ++$i) {
$a = true ? $a : $a;
}
Without the patch, this runs in more than a minute. With the patch, this
runs in less than 0.01 second.
I copied most of the code from the ZEND_RETURN handler; I'm not sure why
IS_CONST are copied as well ? (can't we prevent the IS_CONST from being
modified by incrementing its refcount ?)
Best regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/NEWS branches/PHP_5_3/Zend/tests/bug53727.phpt branches/PHP_5_3/Zend/tests/is_a.phpt branches/PHP_5_3/Zend/zend_builtin_functions.c bran
Hi Stas,
On 07/11/2011 11:25 AM, Stas Malyshev wrote:
Hi!
On 7/11/11 12:18 AM, Dmitry Stogov wrote:
Hi Ralph,
I don't have strong opinion about your suggestion.
In some cases this warning might help to find a typo, in others it may
be useless, but anyway, I think that usage of undefined class name is a
quite rare situation. I would prefer to keep it as is.
It's not as rare, if people use is_a the same way as instanceof. Example
I've seen recently - suppose you've got a method that receives either
instance of an object or it's ID, something like this:
function process($obj)
{
if(!is_a($obj, MyClass) {
$obj = new MyClass($obj);
}
// process $obj
}
Here you've got a new warning. Of course, you could do just $obj
instanceof MyClass, but it's a very subtle distinction which many people
miss. So the thing here is not that $obj has unknown class name - it's
not even a class name at all.
That warning might be only emitted if the first operand is a name of non
existing class.
is_a(NonExistingClass, MyClass);
I think this warning may make sense. :)
Thanks. Dmitry.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Ternary operator improvements
For now the patch still looks a bit messing to me. Also, I would probably prefer to use new opcodes ZEND_QM_ASSIGN_VAR and ZEND_JMP_SET_VAR. Ok, I'll change the patch to use new opcodes instead It's not necessary yet. Please, let me think a bit more. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Fix for #53727 (is_subclass_of resolution with interfaces)
Hi Ralph,
Your patch was absolutely correct.
I've committed it with small improvements.
Thanks. Dmitry.
On 07/01/2011 02:48 AM, Ralph Schindler wrote:
Hey Dmitry,
is_a_impl() is an implementation that is shared by is_a() and
is_subclass_of() calls. There is a flag called only_subclass that allows
there to be switching behavior in is_a_impl().
The issue has been updated with a new patch that now takes objects into
account. It also has a comment in there that better explains what the
patch is doing.
I cannot assign it to you as I am not the original reporter, nor have
access, can you assign it to yourself?
Thanks,
Ralph
On 6/30/11 2:39 AM, Dmitry Stogov wrote:
Hi Ralph,
I agree with expected behavior, but didn't understand the patch and
which side effects it may have as well as the original implementation of
is_a_impl(). It looks for me like one huge bug :)
Please reassign the bug to me, if you like me to take a deeper look.
I can do it only on next week.
Thanks. Dmitry.
On 06/29/2011 11:39 PM, Ralph Schindler wrote:
Hi all,
Concerning: https://bugs.php.net/bug.php?id=53727
I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:
interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true
It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:
interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true
Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.
I've added a test file and all the current Zend/tests run as expected.
Thanks,
Ralph
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Fix for #53727 (is_subclass_of resolution with interfaces)
Hi Ralph,
I reassigned it myself, and will take a look on Monday or Tuesday.
Thanks. Dmitry.
On 07/01/2011 02:48 AM, Ralph Schindler wrote:
Hey Dmitry,
is_a_impl() is an implementation that is shared by is_a() and
is_subclass_of() calls. There is a flag called only_subclass that allows
there to be switching behavior in is_a_impl().
The issue has been updated with a new patch that now takes objects into
account. It also has a comment in there that better explains what the
patch is doing.
I cannot assign it to you as I am not the original reporter, nor have
access, can you assign it to yourself?
Thanks,
Ralph
On 6/30/11 2:39 AM, Dmitry Stogov wrote:
Hi Ralph,
I agree with expected behavior, but didn't understand the patch and
which side effects it may have as well as the original implementation of
is_a_impl(). It looks for me like one huge bug :)
Please reassign the bug to me, if you like me to take a deeper look.
I can do it only on next week.
Thanks. Dmitry.
On 06/29/2011 11:39 PM, Ralph Schindler wrote:
Hi all,
Concerning: https://bugs.php.net/bug.php?id=53727
I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:
interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true
It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:
interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true
Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.
I've added a test file and all the current Zend/tests run as expected.
Thanks,
Ralph
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Fix for #53727 (is_subclass_of resolution with interfaces)
Hi Ralph,
I agree with expected behavior, but didn't understand the patch and
which side effects it may have as well as the original implementation of
is_a_impl(). It looks for me like one huge bug :)
Please reassign the bug to me, if you like me to take a deeper look.
I can do it only on next week.
Thanks. Dmitry.
On 06/29/2011 11:39 PM, Ralph Schindler wrote:
Hi all,
Concerning: https://bugs.php.net/bug.php?id=53727
I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:
interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true
It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:
interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true
Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.
I've added a test file and all the current Zend/tests run as expected.
Thanks,
Ralph
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] $arr = array('Hello', 'world'); $arr();
Hi Felipe,
I like the idea. It makes indirect method calls less expensive.
I would add a hint to specializer, to eliminate small overhead for
regular function calls.
} else if (OP1_TYPE != IS_CONST
EXPECTED(Z_TYPE_P(function_name) == IS_ARRAY)
zend_hash_num_elements(Z_ARRVAL_P(function_name)) == 2) {
Thanks. Dmitry.
On 06/05/2011 07:52 PM, Felipe Pena wrote:
Hi all,
Reading our bug tracker I noticed a good feature request [1] from 2009 which
points to an interesting feature that I think makes sense for us, since we
are now working with $f() using objects and strings, and the array('class',
'method') is an old known for call_user_func()-like functions.
So, I wrote a patch [2] that allow such behavior to be consistent with
arrays. See some examples:
class Hello {
public function world($x) {
echo Hello, $x\n; return $this;
}
}
$f = array('Hello','world');
var_dump($f('you'));
$f = array(new Hello, 'foo');
$f();
All such calls match with the call_user_func() behavior related to magic
methods, static non-static methods.
The array to be a valid callback should be a 2-element array, and it must be
for the first element object/string and for the second string only. (just
like our zend_is_callable() check and opcodes related to init call)
Any thoughts?
[1] - http://bugs.php.net/bug.php?id=47160
[2] - http://felipe.ath.cx/diff/fr47160.diff
phpt: http://felipe.ath.cx/diff/fr47160.phpt
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Final version, RFC release process
Hi, In my opinion a restriction API compatibility must be kept (internals and userland) for x.y.z to x.y+1.z is too strict. It just can block some new features forever. I would suggest to change API compatibility must be kept to API backward compatibility must be kept as much as possible. Thanks. Dmitry. On 06/01/2011 02:09 PM, Pierre Joye wrote: Hi, Long due but finally sending the final RFC for the release process. There a couple of changes since the last time, they are all about making it more transparent or catch the edge cases. We also got new proposers on board, we are now basically almost all active devs on board. URL: https://wiki.php.net/rfc/releaseprocess In parallel to the current discussions about 5.4's features, let get this RFC approved and use right away for 5.3 (to define its life time) and 5.4. That will spare us the issues we had in the past with 5.3, for example and ensure that we get 5.4 release in a reasonable timeframe and without endless delays, or features additions. It is important to keep in mind that this RFC is not written in stone but it is a good base to begin with. We can always adapt it later, if necessary. https://wiki.php.net/rfc/releaseprocess/vote for the votes, pls add your svn id. Proposers do not need to do it as they are indeed there as +1 already (I added myself as example) Cheers, -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Final version, RFC release process
Before now each major (5.y+1) release introduced API changes. We just couldn't introduce literal tables, interned strings, etc without API changes. However such API breaks where invisible for user-land and most extensions, they required a lot of changes in O+, APC, xdebug, etc. But, in case we freeze the API we just won't be able to add many future improvements. Thanks. Dmitry. On 06/01/2011 03:19 PM, John Crenshaw wrote: That isn't measurable, so it is a suggestion, not a standard. It also creates serious problems in userland if APIs change. API changes lead hosts to literally take years to update to new versions of PHP, for fear of breaking the sites that host with them. What about: Userland API compatibility of documented interfaces and behaviors must be kept. API internals should be backwards compatible wherever possible. This relaxes the userland restriction just slightly to allow for changes that break undocumented behaviors, but leaves it basically stable and measurable. This also leaves the door open for internal changes if they're really needed, but basically suggests against it. John Crenshaw Priacta, Inc. -Original Message- From: Dmitry Stogov [mailto:dmi...@zend.com] Sent: Wednesday, June 01, 2011 7:08 AM To: Pierre Joye Cc: PHP internals Subject: Re: [PHP-DEV] Final version, RFC release process Hi, In my opinion a restriction API compatibility must be kept (internals and userland) for x.y.z to x.y+1.z is too strict. It just can block some new features forever. I would suggest to change API compatibility must be kept to API backward compatibility must be kept as much as possible. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] arithmetic speedup
Hi,
The attached patch improves speed of numeric operations by inlining the
most probable paths directy into executor. It also optimizes some
operations for x86 CPU using assembler.
The bench.php gets more than 10% speedup (2.5 sec instead of 2.9 sec)
Real-life applications are not affected. All the PHPT tests are passed.
I'm going to commit the patch on next week if no objections.
Any related ideas are welcome.
Thanks. Dmitry.
Index: Zend/zend_multiply.h
===
--- Zend/zend_multiply.h(revision 311277)
+++ Zend/zend_multiply.h(working copy)
@@ -31,6 +31,18 @@
else (lval) = __tmpvar;
\
} while (0)
+#elif defined(__x86_64__) defined(__GNUC__)
+
+#define ZEND_SIGNED_MULTIPLY_LONG(a, b, lval, dval, usedval) do { \
+ long __tmpvar;
\
+ __asm__ (imulq %3,%0\n
\
+ adcq $0,%1
\
+ : =r(__tmpvar),=r(usedval)
\
+ : 0(a), r(b), 1(0));
\
+ if (usedval) (dval) = (double) (a) * (double) (b);
\
+ else (lval) = __tmpvar;
\
+} while (0)
+
#elif SIZEOF_LONG == 4 defined(HAVE_ZEND_LONG64)
#define ZEND_SIGNED_MULTIPLY_LONG(a, b, lval, dval, usedval) do { \
Index: Zend/zend_operators.c
===
--- Zend/zend_operators.c (revision 311277)
+++ Zend/zend_operators.c (working copy)
@@ -27,13 +27,10 @@
#include zend_globals.h
#include zend_list.h
#include zend_API.h
-#include zend_multiply.h
#include zend_strtod.h
#include zend_exceptions.h
#include zend_closures.h
-#define LONG_SIGN_MASK (1L (8*sizeof(long)-1))
-
#if ZEND_USE_TOLOWER_L
#include locale.h
static _locale_t current_locale = NULL;
Index: Zend/zend_operators.h
===
--- Zend/zend_operators.h (revision 311277)
+++ Zend/zend_operators.h (working copy)
@@ -31,11 +31,14 @@
#endif
#include zend_strtod.h
+#include zend_multiply.h
#if 0HAVE_BCMATH
#include ext/bcmath/libbcmath/src/bcmath.h
#endif
+#define LONG_SIGN_MASK (1L (8*sizeof(long)-1))
+
BEGIN_EXTERN_C()
ZEND_API int add_function(zval *result, zval *op1, zval *op2 TSRMLS_DC);
ZEND_API int sub_function(zval *result, zval *op1, zval *op2 TSRMLS_DC);
@@ -448,8 +451,418 @@
#define zend_update_current_locale()
#endif
+static zend_always_inline int fast_increment_function(zval *op1)
+{
+ if (EXPECTED(Z_TYPE_P(op1) == IS_LONG)) {
+#if defined(__GNUC__) defined(__i386__)
+ __asm__(
+ incl (%0)\n\t
+ jno 0f\n\t
+ movl $0x0, (%0)\n\t
+ movl $0x41e0, 0x4(%0)\n\t
+ movb $0x2,0xc(%0)\n
+ 0:
+ :
+ : r(op1));
+#elif defined(__GNUC__) defined(__x86_64__)
+ __asm__(
+ incq (%0)\n\t
+ jno 0f\n\t
+ movl $0x0, (%0)\n\t
+ movl $0x43e0, 0x4(%0)\n\t
+ movb $0x2,0x14(%0)\n
+ 0:
+ :
+ : r(op1));
+#else
+ if (UNEXPECTED(Z_LVAL_P(op1) == LONG_MAX)) {
+ /* switch to double */
+ Z_DVAL_P(op1) = (double)LONG_MAX + 1.0;
+ Z_TYPE_P(op1) = IS_DOUBLE;
+ } else {
+ Z_LVAL_P(op1)++;
+ }
#endif
+ return SUCCESS;
+ }
+ return increment_function(op1);
+}
+static zend_always_inline int fast_decrement_function(zval *op1)
+{
+ if (EXPECTED(Z_TYPE_P(op1) == IS_LONG)) {
+#if defined(__GNUC__) defined(__i386__)
+ __asm__(
+ decl (%0)\n\t
+ jno 0f\n\t
+ movl $0x0020, (%0)\n\t
+ movl $0xc1e0, 0x4(%0)\n\t
+ movb $0x2,0xc(%0)\n
+ 0:
+ :
+ : r(op1));
+#elif defined(__GNUC__) defined(__x86_64__)
+ __asm__(
+ decq (%0)\n\t
+ jno 0f\n\t
+ movl $0x, (%0)\n\t
+ movl $0xc3e0,
Re: [PHP-DEV] [PATCH] arithmetic speedup
Hi Pierre, On 05/20/2011 01:49 PM, Pierre Joye wrote: hi Dmitry, Nice improvements, thanks :) Any reason not to have done the changes for windows as well? Sorry, I'm not an expert in MS VC inline assembler. As I remember in VC6 it was poor and didn't allow complicated things. In case someone can add support for VC it would be great. What's about putting the asm code in external file so it can used by more compilers? (some has issues with inline asm, like VC in x64 mode, other may have as well afair). The main idea of the patch is inlining and I don't know how can I inline from external file. In simple cases the function call, parameter passing, prologue, epilogue make more overhead than the opration itself. So the inlining is responsable for 90% of speedup while asm optimization only for 10%. Thanks. Dmitry. Cheers, On Fri, May 20, 2011 at 11:28 AM, Dmitry Stogovdmi...@zend.com wrote: Hi, The attached patch improves speed of numeric operations by inlining the most probable paths directy into executor. It also optimizes some operations for x86 CPU using assembler. The bench.php gets more than 10% speedup (2.5 sec instead of 2.9 sec) Real-life applications are not affected. All the PHPT tests are passed. I'm going to commit the patch on next week if no objections. Any related ideas are welcome. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] arithmetic speedup
On 05/20/2011 02:05 PM, Pierre Joye wrote: On Fri, May 20, 2011 at 12:01 PM, Dmitry Stogovdmi...@zend.com wrote: Hi Pierre, On 05/20/2011 01:49 PM, Pierre Joye wrote: hi Dmitry, Nice improvements, thanks :) Any reason not to have done the changes for windows as well? Sorry, I'm not an expert in MS VC inline assembler. As I remember in VC6 it was poor and didn't allow complicated things. In case someone can add support for VC it would be great. What's about putting the asm code in external file so it can used by more compilers? (some has issues with inline asm, like VC in x64 mode, other may have as well afair). The main idea of the patch is inlining and I don't know how can I inline from external file. In simple cases the function call, parameter passing, prologue, epilogue make more overhead than the opration itself. So the inlining is responsable for 90% of speedup while asm optimization only for 10%. Thanks. Dmitry. As this patch is only GCC (and linux I suppose), The inlining must be done on all platforms so it should affect Windows too. I would suggest to simply commit in trunk right now. Other platforms can implement it later (I will do it for windows in the next weeks). I'll commit on Monday. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Reverting ext/mbstring patch
Hi Moriyoshi,
OK, I thought the email was lost, so ignore the email I just resent.
In general I like your patch and I would glad to see it fixed.
I already tried to make some fixes.
See the attached patch.
Thanks. Dmitry.
On 03/02/2011 11:51 PM, Moriyoshi Koizumi wrote:
Hey,
I think I can fix it somehow. Please don't be haste with it. I am
going to look into it.
Moriyoshi
On Tue, Mar 1, 2011 at 11:35 PM, Dmitry Stogovdmi...@zend.com wrote:
Hi,
I'm going to revert Moriyoshi patch from December and some following fixes.
I like the idea of the patch, but it just doesn't work as expected.
It breaks 10 tests, but in general it breaks most things related to Unicode
(declare statement, multibyte scripts, exif support for Unicode, multibyte
POST requests).
I tried to fix it myself, but I just can't understand how it should work
(it's too big). It also has several places where integers messed with
pointers, old API messed with new one and so on.
I'm going to revert (apply the attached patch) on Thursday.
Following is the list of failed tests:
Shift_JIS request [tests/basic/029.phpt]
Testing declare statement with several type values
[Zend/tests/declare_001.phpt]
Zend Multibyte and ShiftJIS
[Zend/tests/multibyte/multibyte_encoding_001.phpt]
Zend Multibyte and UTF-8 BOM
[Zend/tests/multibyte/multibyte_encoding_002.phpt]
Zend Multibyte and UTF-16 BOM
[Zend/tests/multibyte/multibyte_encoding_003.phpt]
encoding conversion from script encoding into internal encoding
[Zend/tests/multibyte/multibyte_encoding_005.phpt]
086: bracketed namespace with encoding [Zend/tests/ns_086.phpt]
Check for exif_read_data, Unicode user comment [ext/exif/tests/exif003.phpt]
Check for exif_read_data, Unicode WinXP tags [ext/exif/tests/exif004.phpt]
Test mb_get_info() function [ext/mbstring/tests/mb_get_info.phpt]
Thanks. Dmitry.
Index: ext/exif/exif.c
===
--- ext/exif/exif.c (revision 308813)
+++ ext/exif/exif.c (working copy)
@@ -2664,13 +2664,13 @@
decode = ImageInfo-decode_unicode_le;
}
if (zend_multibyte_encoding_converter(
- pszInfoPtr,
+ (unsigned char**)pszInfoPtr,
len,
- szValuePtr,
+ (unsigned char*)szValuePtr,
ByteCount,
- ImageInfo-encode_unicode,
- decode
- TSRMLS_CC) != 0) {
+ zend_multibyte_fetch_encoding(ImageInfo-encode_unicode TSRMLS_CC),
+ zend_multibyte_fetch_encoding(decode TSRMLS_CC)
+ TSRMLS_CC) 0) {
len = exif_process_string_raw(pszInfoPtr, szValuePtr, ByteCount);
}
return len;
@@ -2684,13 +2684,13 @@
szValuePtr = szValuePtr+8;
ByteCount -= 8;
if (zend_multibyte_encoding_converter(
- pszInfoPtr,
+ (unsigned char**)pszInfoPtr,
len,
- szValuePtr,
+ (unsigned char*)szValuePtr,
ByteCount,
- ImageInfo-encode_jis,
- ImageInfo-motorola_intel ? ImageInfo-decode_jis_be : ImageInfo-decode_jis_le
- TSRMLS_CC) != 0) {
+ zend_multibyte_fetch_encoding(ImageInfo-encode_jis TSRMLS_CC),
+ zend_multibyte_fetch_encoding(ImageInfo-motorola_intel ? ImageInfo-decode_jis_be : ImageInfo-decode_jis_le TSRMLS_CC)
+ TSRMLS_CC) 0) {
len = exif_process_string_raw(pszInfoPtr, szValuePtr, ByteCount);
}
return len;
@@ -2723,13 +2723,13 @@
/* Copy the comment */
if (zend_multibyte_encoding_converter(
- xp_field-value,
+ (unsigned char**)xp_field-value,
xp_field-size,
- szValuePtr,
+ (unsigned char*)szValuePtr,
ByteCount,
- ImageInfo-encode_unicode,
- ImageInfo-motorola_intel ? ImageInfo-decode_unicode_be : ImageInfo-decode_unicode_le
- TSRMLS_CC) != 0) {
+ zend_multibyte_fetch_encoding(ImageInfo-encode_unicode TSRMLS_CC),
+ zend_multibyte_fetch_encoding(ImageInfo-motorola_intel ? ImageInfo-decode_unicode_be : ImageInfo-decode_unicode_le TSRMLS_CC)
+ TSRMLS_CC) 0) {
xp_field-size = exif_process_string_raw(xp_field-value, szValuePtr, ByteCount);
}
return xp_field-size;
Index: ext/mbstring/tests/mb_encoding_aliases.phpt
===
--- ext/mbstring/tests/mb_encoding_aliases.phpt (revision 308813)
+++ ext/mbstring/tests/mb_encoding_aliases.phpt (working copy)
@@ -13,26 +13,28 @@
?
--EXPECTF--
Warning: mb_encoding_aliases() expects exactly 1 parameter, 0 given in %s on line 2
-array(10) {
+array(11) {
[0]=
string(14) ANSI_X3.4-1968
[1]=
string(14) ANSI_X3.4-1986
[2]=
+ string(7) IBM-367
+ [3]=
string(6) IBM367
- [3]=
+ [4]=
string(9) ISO646-US
- [4]=
+ [5]=
string(16) ISO_646.irv:1991
- [5]=
+ [6]=
string(8) US-ASCII
- [6]=
+ [7]=
string(5) cp367
- [7]=
+ [8]=
string(7) csASCII
- [8]=
+ [9]=
string(8) iso-ir-6
- [9]=
+ [10]=
string(2) us
}
array(0) {
Index: ext/mbstring/mbstring.c
===
--- ext/mbstring/mbstring.c (revision 308813)
+++ ext/mbstring/mbstring.c (working copy)
@@ -2910,7
[PHP-DEV] ext/exif and Zend/tests/multibyte tests are still broken
Hi Moriyoshi, Some ext/exif and Zend/tests/multibyte tests are still broken after your mbstring related changes on December. ext/exif just can't work because you changed the prototypes of zend_multibyte functions but not the function invocations. Are you going to fix it? Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] ext/mbstring dependencies
Hi,
The proposed patch completely removes ext/mbstring compile-time
dependencies. As result the same php binaries may be used for Asian and
European languages without performance degradation. ext/mbstring now may
be compiled as a DSO. I'm going to commit the patch on Wednesday.
Any comments are welcome.
Thanks. Dmitry.
Index: ext/standard/html.c
===
--- ext/standard/html.c (revision 305821)
+++ ext/standard/html.c (working copy)
@@ -54,11 +54,6 @@
#include langinfo.h
#endif
-#if HAVE_MBSTRING
-# include ext/mbstring/mbstring.h
-ZEND_EXTERN_MODULE_GLOBALS(mbstring)
-#endif
-
#include zend_hash.h
#include html_tables.h
@@ -372,7 +367,6 @@
int i;
enum entity_charset charset = cs_utf_8;
int len = 0;
- zval *uf_result = NULL;
/* Default is now UTF-8 */
if (charset_hint == NULL)
@@ -381,80 +375,12 @@
if ((len = strlen(charset_hint)) != 0) {
goto det_charset;
}
-#if HAVE_MBSTRING
-#if !defined(COMPILE_DL_MBSTRING)
- /* XXX: Ugly things. Why don't we look for a more sophisticated way? */
- switch (MBSTRG(current_internal_encoding)) {
- case mbfl_no_encoding_8859_1:
- return cs_8859_1;
- case mbfl_no_encoding_utf8:
- return cs_utf_8;
-
- case mbfl_no_encoding_euc_jp:
- case mbfl_no_encoding_eucjp_win:
- return cs_eucjp;
-
- case mbfl_no_encoding_sjis:
- case mbfl_no_encoding_sjis_open:
- case mbfl_no_encoding_cp932:
- return cs_sjis;
-
- case mbfl_no_encoding_cp1252:
- return cs_cp1252;
-
- case mbfl_no_encoding_8859_15:
- return cs_8859_15;
-
- case mbfl_no_encoding_big5:
- return cs_big5;
-
- case mbfl_no_encoding_euc_cn:
- case mbfl_no_encoding_hz:
- case mbfl_no_encoding_cp936:
- return cs_gb2312;
-
- case mbfl_no_encoding_koi8r:
- return cs_koi8r;
-
- case mbfl_no_encoding_cp866:
- return cs_cp866;
-
- case mbfl_no_encoding_cp1251:
- return cs_cp1251;
-
- case mbfl_no_encoding_8859_5:
- return cs_8859_5;
-
- default:
- ;
+ charset_hint = (char*)zend_multibyte_get_internal_encoding(TSRMLS_C);
+ if (charset_hint != NULL (len=strlen(charset_hint)) != 0) {
+ goto det_charset;
}
-#else
- {
- zval nm_mb_internal_encoding;
- ZVAL_STRING(nm_mb_internal_encoding, mb_internal_encoding,
0);
-
- if (call_user_function_ex(CG(function_table), NULL,
nm_mb_internal_encoding, uf_result, 0, NULL, 1, NULL TSRMLS_CC) != FAILURE) {
-
- charset_hint = Z_STRVAL_P(uf_result);
- len = Z_STRLEN_P(uf_result);
-
- if ((len == 4) /* sizeof(none|auto|pass)-1 */
- (!memcmp(pass, charset_hint,
sizeof(pass) - 1) ||
- !memcmp(auto, charset_hint,
sizeof(auto) - 1) ||
- !memcmp(none, charset_hint,
sizeof(none) - 1))) {
-
- charset_hint = NULL;
- len = 0;
- } else {
- goto det_charset;
- }
- }
- }
-#endif
-#endif
-
charset_hint = SG(default_charset);
if (charset_hint != NULL (len=strlen(charset_hint)) != 0) {
goto det_charset;
@@ -514,9 +440,6 @@
charset_hint);
}
}
- if (uf_result != NULL) {
- zval_ptr_dtor(uf_result);
- }
return charset;
}
/* }}} */
Index: ext/exif/exif.c
===
--- ext/exif/exif.c (revision 305821)
+++ ext/exif/exif.c (working copy)
@@ -66,16 +66,6 @@
#include ext/standard/php_image.h
#include ext/standard/info.h
-#if defined(PHP_WIN32) || (HAVE_MBSTRING !defined(COMPILE_DL_MBSTRING))
-#define EXIF_USE_MBSTRING 1
-#else
-#define EXIF_USE_MBSTRING 0
-#endif
-
-#if EXIF_USE_MBSTRING
-#include ext/mbstring/mbstring.h
-#endif
-
/* needed for ssize_t definition */
#include sys/types.h
@@ -176,23 +166,19 @@
ZEND_INI_MH(OnUpdateEncode)
{
-#if EXIF_USE_MBSTRING
- if (new_value strlen(new_value)
!php_mb_check_encoding_list(new_value TSRMLS_CC)) {
+ if (new_value strlen(new_value)
!zend_multibyte_check_encoding_list(new_value
Re: [PHP-DEV] ext/mbstring dependencies
Thanks. Already removed it.
Dmitry.
On 12/06/2010 01:21 PM, Hannes Magnússon wrote:
On Mon, Dec 6, 2010 at 09:31, Dmitry Stogovdmi...@zend.com wrote:
Hi,
The proposed patch completely removes ext/mbstring compile-time
dependencies. As result the same php binaries may be used for Asian and
European languages without performance degradation. ext/mbstring now may be
compiled as a DSO. I'm going to commit the patch on Wednesday.
Any comments are welcome.
Random scrolling through the patch:
Index: ext/mbstring/mbstring.c
+// to = php_mb_convert_encoding(from, from_len, encoding_to,
encoding_from, to_length);
+// return to ? 0 : -1;
and WS
+!memcmp(none, name, sizeof(none) - 1 {
-Hannes
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: --enable-zend-multibyte
Hi Moriyoshi, On 12/06/2010 01:31 PM, Moriyoshi Koizumi wrote: Hi, How about using the value of mbstring.script_encoding to determine whether to enable the encoding conversion feature? If the value is the same as that of mbstring.internal_encoding, then no conversion should be needed in the first place. Besides we can define some singular value like none that completely disables the conversion. Right now I introduced zend.multibyte directive which enables to look into mbstring.script_encoding and mbstring.internal_encoding as it did before with --enable-zend-multibyte. Note that we can't check for them in ZE directly because ZE knows nothing about ext/mbstring. It may be not compiled into PHP or compiled as DSO. Probably it's possible to do through additional callbacks. Regarding the dependency on mbstring extension, I think it's time to enable mbstring by default. The idea I'm working on is to provide an ability to enable/disable all multibyte features without PHP recompilation. So the same binaries will be able to support Asian languages and work with European without performance degradation. My second patch adds the missing parts (POST request parsing, htmlentities, EXIF). I sent it to internals@ today. Thanks. Dmitry. Regards, Moriyoshi -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] ext/mbstring dependencies
On 12/06/2010 08:38 PM, Moriyoshi Koizumi wrote: Hi, The patch almost looks good to me, but we should be more careful about introducing a set of hook points into the API. I think it'd be great if the multipart parser portion was rewritten so that it would only call the Zend multibyte API's despite a slight performance drawback. Really I thought in the same way, but finally I decided not to put RFC1867 specific callbacks into zend engine, because ZE does nothing related to RFC1867 handling at all. Thanks. Dmitry. Regards, Moriyoshi On Mon, Dec 6, 2010 at 5:31 PM, Dmitry Stogovdmi...@zend.com wrote: Hi, The proposed patch completely removes ext/mbstring compile-time dependencies. As result the same php binaries may be used for Asian and European languages without performance degradation. ext/mbstring now may be compiled as a DSO. I'm going to commit the patch on Wednesday. Any comments are welcome. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: --enable-zend-multibyte
On 12/06/2010 08:25 PM, Moriyoshi Koizumi wrote: On Mon, Dec 6, 2010 at 7:49 PM, Dmitry Stogovdmi...@zend.com wrote: Hi Moriyoshi, On 12/06/2010 01:31 PM, Moriyoshi Koizumi wrote: Hi, How about using the value of mbstring.script_encoding to determine whether to enable the encoding conversion feature? If the value is the same as that of mbstring.internal_encoding, then no conversion should be needed in the first place. Besides we can define some singular value like none that completely disables the conversion. Right now I introduced zend.multibyte directive which enables to look into mbstring.script_encoding and mbstring.internal_encoding as it did before with --enable-zend-multibyte. Note that we can't check for them in ZE directly because ZE knows nothing about ext/mbstring. It may be not compiled into PHP or compiled as DSO. Probably it's possible to do through additional callbacks. Indeed mbstring.script_encoding is only used by zend_multibyte, so it would make sense to alter the name of the ini setting to something like zend.script_encoding, and move the relevant part of mbstring.c into zend_multibyte.c Usage of zend.script_encoding instead of zend.multibyte might make sense. I'll take a look into it. However I don't see a way to move the relevant parts of ext/mbstring into zend_multibute.c. zend_multibyte will have to call for external detector and converter anyway. May be I misunderstood you. Could you explain what you means? Thanks. Dmitry. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: --enable-zend-multibyte
On 12/07/2010 03:32 AM, Moriyoshi Koizumi wrote: On Tue, Dec 7, 2010 at 5:57 AM, Dmitry Stogovdmi...@zend.com wrote: On 12/06/2010 08:25 PM, Moriyoshi Koizumi wrote: On Mon, Dec 6, 2010 at 7:49 PM, Dmitry Stogovdmi...@zend.comwrote: Hi Moriyoshi, On 12/06/2010 01:31 PM, Moriyoshi Koizumi wrote: Hi, How about using the value of mbstring.script_encoding to determine whether to enable the encoding conversion feature? If the value is the same as that of mbstring.internal_encoding, then no conversion should be needed in the first place. Besides we can define some singular value like none that completely disables the conversion. Right now I introduced zend.multibyte directive which enables to look into mbstring.script_encoding and mbstring.internal_encoding as it did before with --enable-zend-multibyte. Note that we can't check for them in ZE directly because ZE knows nothing about ext/mbstring. It may be not compiled into PHP or compiled as DSO. Probably it's possible to do through additional callbacks. Indeed mbstring.script_encoding is only used by zend_multibyte, so it would make sense to alter the name of the ini setting to something like zend.script_encoding, and move the relevant part of mbstring.c into zend_multibyte.c Usage of zend.script_encoding instead of zend.multibyte might make sense. I'll take a look into it. However I don't see a way to move the relevant parts of ext/mbstring into zend_multibute.c. zend_multibyte will have to call for external detector and converter anyway. May be I misunderstood you. Could you explain what you means? I meant the part only related to the ini setting. Related part in mb_get_info() can be removed as it would no longer belong to mbstring settings. While I am not sure of what you meant by external detector and converter, encoding detector would be supplied through zend_multibyte_set_functions() in mbstring's MINIT and there should be no need to bring any extra facility into zend_multibyte.c . OK. then we have the same view. I'll take a look how it's easy to move zend.script_encoding into ZE. Thanks. Dmitry. Regards, Moriyoshi Thanks. Dmitry. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] new foo()-bar()
Hi Felipe,
I'm wondered it works out of the box with so small patches :)
However, both patches introduce new parser conflicts and it would be
grate to avoid them.
Also the patches need to be checked for memory leaks in case of
exceptions thrown from constructor and chained function(s).
It also probably makes sense to add array deference chaining e.g. new
Foo()[] (just for language consistency).
Thanks. Dmitry.
On 11/26/2010 10:36 PM, Felipe Pena wrote:
Hi all,
I'm here again to presents another proposal, which adds support for
instantiating a class and calling its methods and accessing its properties
on same command.
Example:
?php
class bar {
public $x = 'PHP';
}
class foo extends bar {
public function bar() {
return $this;
}
}
var_dump(new foo()-bar()-x); // string(3) PHP
?
Other examples which describes the feature at
http://wiki.php.net/rfc/instance-method-call
Thoughts?
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] break/continue $var
the proposed patch (backport from php6) is attached.
I think this feature is completely useless and it was agreed to remove
it about 5 years ago.
Derick, can I commit the patch today?
Thanks. Dmitry.
Dmitry Stogov wrote:
Hi,
Previously we decided to remove break/continue $var syntax.
I even implemented it in PHP6 brunch, however it wasn't backported into
trunk. Could I do it?
Thanks. Dmitry.
Index: Zend/zend_execute.c
===
--- Zend/zend_execute.c (revision 305710)
+++ Zend/zend_execute.c (working copy)
@@ -1362,21 +1362,12 @@
}
}
-static inline zend_brk_cont_element* zend_brk_cont(zval *nest_levels_zval, int
array_offset, const zend_op_array *op_array, const temp_variable *Ts TSRMLS_DC)
+static inline zend_brk_cont_element* zend_brk_cont(int nest_levels, int
array_offset, const zend_op_array *op_array, const temp_variable *Ts TSRMLS_DC)
{
zval tmp;
- int nest_levels, original_nest_levels;
+ int original_nest_levels = nest_levels;
zend_brk_cont_element *jmp_to;
- if (nest_levels_zval-type != IS_LONG) {
- tmp = *nest_levels_zval;
- zval_copy_ctor(tmp);
- convert_to_long(tmp);
- nest_levels = tmp.value.lval;
- } else {
- nest_levels = nest_levels_zval-value.lval;
- }
- original_nest_levels = nest_levels;
do {
if (array_offset==-1) {
zend_error_noreturn(E_ERROR, Cannot break/continue %d
level%s, original_nest_levels, (original_nest_levels == 1) ? : s);
Index: Zend/zend_vm_execute.h
===
--- Zend/zend_vm_execute.h (revision 305710)
+++ Zend/zend_vm_execute.h (working copy)
@@ -1304,11 +1304,10 @@
static int ZEND_FASTCALL ZEND_BRK_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
{
USE_OPLINE
-
zend_brk_cont_element *el;
SAVE_OPLINE();
- el = zend_brk_cont(opline-op2.zv, opline-op1.opline_num,
+ el = zend_brk_cont(Z_LVAL_P(opline-op2.zv), opline-op1.opline_num,
EX(op_array), EX_Ts() TSRMLS_CC);
ZEND_VM_JMP(EX(op_array)-opcodes + el-brk);
@@ -1317,11 +1316,10 @@
static int ZEND_FASTCALL
ZEND_CONT_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
{
USE_OPLINE
-
zend_brk_cont_element *el;
SAVE_OPLINE();
- el = zend_brk_cont(opline-op2.zv, opline-op1.opline_num,
+ el = zend_brk_cont(Z_LVAL_P(opline-op2.zv), opline-op1.opline_num,
EX(op_array), EX_Ts() TSRMLS_CC);
ZEND_VM_JMP(EX(op_array)-opcodes + el-cont);
@@ -1334,7 +1332,7 @@
zend_brk_cont_element *el;
SAVE_OPLINE();
- el = zend_brk_cont(opline-op2.zv, opline-extended_value,
+ el = zend_brk_cont(Z_LVAL_P(opline-op2.zv), opline-extended_value,
EX(op_array), EX_Ts() TSRMLS_CC);
brk_opline = EX(op_array)-opcodes + el-brk;
@@ -1477,32 +1475,6 @@
}
-static int ZEND_FASTCALL ZEND_BRK_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
-{
- USE_OPLINE
- zend_free_op free_op2;
- zend_brk_cont_element *el;
-
- SAVE_OPLINE();
- el = zend_brk_cont(_get_zval_ptr_tmp(opline-op2.var, EX_Ts(),
free_op2 TSRMLS_CC), opline-op1.opline_num,
- EX(op_array), EX_Ts() TSRMLS_CC);
- zval_dtor(free_op2.var);
- ZEND_VM_JMP(EX(op_array)-opcodes + el-brk);
-}
-
-static int ZEND_FASTCALL ZEND_CONT_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
-{
- USE_OPLINE
- zend_free_op free_op2;
- zend_brk_cont_element *el;
-
- SAVE_OPLINE();
- el = zend_brk_cont(_get_zval_ptr_tmp(opline-op2.var, EX_Ts(),
free_op2 TSRMLS_CC), opline-op1.opline_num,
- EX(op_array), EX_Ts() TSRMLS_CC);
- zval_dtor(free_op2.var);
- ZEND_VM_JMP(EX(op_array)-opcodes + el-cont);
-}
-
static int ZEND_FASTCALL
ZEND_FETCH_CLASS_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
{
USE_OPLINE
@@ -1599,32 +1571,6 @@
}
-static int ZEND_FASTCALL ZEND_BRK_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
-{
- USE_OPLINE
- zend_free_op free_op2;
- zend_brk_cont_element *el;
-
- SAVE_OPLINE();
- el = zend_brk_cont(_get_zval_ptr_var(opline-op2.var, EX_Ts(),
free_op2 TSRMLS_CC), opline-op1.opline_num,
- EX(op_array), EX_Ts() TSRMLS_CC);
- if (free_op2.var) {zval_ptr_dtor(free_op2.var);};
- ZEND_VM_JMP(EX(op_array)-opcodes + el-brk);
-}
-
-static int ZEND_FASTCALL ZEND_CONT_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
-{
- USE_OPLINE
- zend_free_op free_op2;
- zend_brk_cont_element *el;
-
- SAVE_OPLINE();
- el = zend_brk_cont(_get_zval_ptr_var(opline-op2.var, EX_Ts(),
free_op2 TSRMLS_CC), opline-op1.opline_num,
- EX(op_array), EX_Ts() TSRMLS_CC
Re: [PHP-DEV] break/continue $var
Pierre Joye wrote:
Hi Dmitry,
From a BC point of view, the impact will be minimal. The only project
I found relying on this syntax is XPath class [1].
I'm wondered why they use it, because their var ($parseBlock) is always 1.
Please add a note the upgrade guide as well in trunk, maybe in a BC
break sections.
done.
Thanks. Dmitry.
[1] http://google.com/codesearch?hl=ensa=Nq=lang:php+break\s{1,}%28\%24\w%29
Cheers,
On Wed, Nov 24, 2010 at 1:11 PM, Dmitry Stogov dmi...@zend.com wrote:
the proposed patch (backport from php6) is attached.
I think this feature is completely useless and it was agreed to remove it
about 5 years ago.
Derick, can I commit the patch today?
Thanks. Dmitry.
Dmitry Stogov wrote:
Hi,
Previously we decided to remove break/continue $var syntax.
I even implemented it in PHP6 brunch, however it wasn't backported into
trunk. Could I do it?
Thanks. Dmitry.
Index: Zend/zend_execute.c
===
--- Zend/zend_execute.c (revision 305710)
+++ Zend/zend_execute.c (working copy)
@@ -1362,21 +1362,12 @@
}
}
-static inline zend_brk_cont_element* zend_brk_cont(zval *nest_levels_zval,
int array_offset, const zend_op_array *op_array, const temp_variable *Ts
TSRMLS_DC)
+static inline zend_brk_cont_element* zend_brk_cont(int nest_levels, int
array_offset, const zend_op_array *op_array, const temp_variable *Ts
TSRMLS_DC)
{
zval tmp;
- int nest_levels, original_nest_levels;
+ int original_nest_levels = nest_levels;
zend_brk_cont_element *jmp_to;
- if (nest_levels_zval-type != IS_LONG) {
- tmp = *nest_levels_zval;
- zval_copy_ctor(tmp);
- convert_to_long(tmp);
- nest_levels = tmp.value.lval;
- } else {
- nest_levels = nest_levels_zval-value.lval;
- }
- original_nest_levels = nest_levels;
do {
if (array_offset==-1) {
zend_error_noreturn(E_ERROR, Cannot break/continue
%d level%s, original_nest_levels, (original_nest_levels == 1) ? : s);
Index: Zend/zend_vm_execute.h
===
--- Zend/zend_vm_execute.h (revision 305710)
+++ Zend/zend_vm_execute.h (working copy)
@@ -1304,11 +1304,10 @@
static int ZEND_FASTCALL
ZEND_BRK_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
{
USE_OPLINE
-
zend_brk_cont_element *el;
SAVE_OPLINE();
- el = zend_brk_cont(opline-op2.zv, opline-op1.opline_num,
+ el = zend_brk_cont(Z_LVAL_P(opline-op2.zv), opline-op1.opline_num,
EX(op_array), EX_Ts() TSRMLS_CC);
ZEND_VM_JMP(EX(op_array)-opcodes + el-brk);
@@ -1317,11 +1316,10 @@
static int ZEND_FASTCALL
ZEND_CONT_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
{
USE_OPLINE
-
zend_brk_cont_element *el;
SAVE_OPLINE();
- el = zend_brk_cont(opline-op2.zv, opline-op1.opline_num,
+ el = zend_brk_cont(Z_LVAL_P(opline-op2.zv), opline-op1.opline_num,
EX(op_array), EX_Ts() TSRMLS_CC);
ZEND_VM_JMP(EX(op_array)-opcodes + el-cont);
@@ -1334,7 +1332,7 @@
zend_brk_cont_element *el;
SAVE_OPLINE();
- el = zend_brk_cont(opline-op2.zv, opline-extended_value,
+ el = zend_brk_cont(Z_LVAL_P(opline-op2.zv), opline-extended_value,
EX(op_array), EX_Ts() TSRMLS_CC);
brk_opline = EX(op_array)-opcodes + el-brk;
@@ -1477,32 +1475,6 @@
}
-static int ZEND_FASTCALL
ZEND_BRK_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
-{
- USE_OPLINE
- zend_free_op free_op2;
- zend_brk_cont_element *el;
-
- SAVE_OPLINE();
- el = zend_brk_cont(_get_zval_ptr_tmp(opline-op2.var, EX_Ts(),
free_op2 TSRMLS_CC), opline-op1.opline_num,
- EX(op_array), EX_Ts() TSRMLS_CC);
- zval_dtor(free_op2.var);
- ZEND_VM_JMP(EX(op_array)-opcodes + el-brk);
-}
-
-static int ZEND_FASTCALL
ZEND_CONT_SPEC_TMP_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
-{
- USE_OPLINE
- zend_free_op free_op2;
- zend_brk_cont_element *el;
-
- SAVE_OPLINE();
- el = zend_brk_cont(_get_zval_ptr_tmp(opline-op2.var, EX_Ts(),
free_op2 TSRMLS_CC), opline-op1.opline_num,
- EX(op_array), EX_Ts() TSRMLS_CC);
- zval_dtor(free_op2.var);
- ZEND_VM_JMP(EX(op_array)-opcodes + el-cont);
-}
-
static int ZEND_FASTCALL
ZEND_FETCH_CLASS_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
{
USE_OPLINE
@@ -1599,32 +1571,6 @@
}
-static int ZEND_FASTCALL
ZEND_BRK_SPEC_VAR_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
-{
- USE_OPLINE
- zend_free_op free_op2;
- zend_brk_cont_element *el;
-
- SAVE_OPLINE();
- el = zend_brk_cont(_get_zval_ptr_var(opline-op2.var, EX_Ts(),
free_op2 TSRMLS_CC), opline-op1.opline_num,
- EX(op_array), EX_Ts() TSRMLS_CC);
- if (free_op2
Re: [PHP-DEV] --enable-zend-multibyte
hi,
I've prepared the patch which completely removes compile-time
--enable-zend-multibyte configuration directive.
The configuration can be changed at run-tume using zend.multibyte=0/1.
The patch also fixes Unicode detection for phar archives.
I would like to commit it on Thursday.
Thanks. Dmitry.
Index: ext/standard/info.c
===
--- ext/standard/info.c (revision 305647)
+++ ext/standard/info.c (working copy)
@@ -759,11 +759,7 @@
php_info_print_table_row(2, Zend Memory Manager,
is_zend_mm(TSRMLS_C) ? enabled : disabled );
-#ifdef ZEND_MULTIBYTE
- php_info_print_table_row(2, Zend Multibyte Support,
enabled);
-#else
- php_info_print_table_row(2, Zend Multibyte Support,
disabled);
-#endif
+ php_info_print_table_row(2, Zend Multibyte Support,
CG(multibyte) ? enabled : disabled);
#if HAVE_IPV6
php_info_print_table_row(2, IPv6 Support, enabled );
Index: ext/phar/tests/zip/notphar.phpt
===
--- ext/phar/tests/zip/notphar.phpt (revision 305647)
+++ ext/phar/tests/zip/notphar.phpt (working copy)
@@ -4,6 +4,7 @@
?php if (!extension_loaded(phar)) die(skip); ?
--INI--
phar.readonly=1
+detect_unicode=0
--FILE--
?php
$fname = dirname(__FILE__) . '/' . basename(__FILE__, '.php') . '.phar.zip';
Index: ext/mbstring/tests/zend_multibyte-10.phpt
===
--- ext/mbstring/tests/zend_multibyte-10.phpt (revision 305647)
+++ ext/mbstring/tests/zend_multibyte-10.phpt (working copy)
@@ -11,6 +11,7 @@
?php
declare(encoding=ISO-8859-15);
declare(encoding=ISO-8859-1);
+echo ok\n;
?
--EXPECTF--
-Fatal error: Encoding declaration pragma must be the very first statement in
the script in %s on line 3
+ok
Index: ext/mbstring/tests/zend_multibyte-11.phpt
===
--- ext/mbstring/tests/zend_multibyte-11.phpt (revision 305647)
+++ ext/mbstring/tests/zend_multibyte-11.phpt (working copy)
@@ -11,7 +11,8 @@
?php
declare(encoding=ISO-8859-15) {
declare(encoding=ISO-8859-1);
+ echo ok\n;
}
?
--EXPECTF--
-Fatal error: Encoding declaration pragma must be the very first statement in
the script in %s on line 3
+ok
Index: ext/mbstring/mbstring.c
===
--- ext/mbstring/mbstring.c (revision 305647)
+++ ext/mbstring/mbstring.c (working copy)
@@ -77,9 +77,7 @@
#include php_mbregex.h
#endif
-#ifdef ZEND_MULTIBYTE
#include zend_multibyte.h
-#endif /* ZEND_MULTIBYTE */
#if HAVE_ONIG
#include php_onig_compat.h
@@ -98,12 +96,10 @@
static PHP_GINIT_FUNCTION(mbstring);
static PHP_GSHUTDOWN_FUNCTION(mbstring);
-#ifdef ZEND_MULTIBYTE
static size_t php_mb_oddlen(const unsigned char *string, size_t length, const
char *encoding TSRMLS_DC);
static int php_mb_encoding_converter(unsigned char **to, size_t *to_length,
const unsigned char *from, size_t from_length, const char *encoding_to, const
char *encoding_from TSRMLS_DC);
static char* php_mb_encoding_detector(const unsigned char *arg_string, size_t
arg_length, char *arg_list TSRMLS_DC);
static int php_mb_set_zend_encoding(TSRMLS_D);
-#endif
/* }}} */
/* {{{ php_mb_default_identify_list */
@@ -1126,12 +1122,14 @@
}
/* }}} */
-#ifdef ZEND_MULTIBYTE
/* {{{ static PHP_INI_MH(OnUpdate_mbstring_script_encoding) */
static PHP_INI_MH(OnUpdate_mbstring_script_encoding)
{
int *list, size;
+ if (!CG(multibyte)) {
+ return FAILURE;
+ }
if (php_mb_parse_encoding_list(new_value, new_value_length, list,
size, 1 TSRMLS_CC)) {
if (MBSTRG(script_encoding_list) != NULL) {
free(MBSTRG(script_encoding_list));
@@ -1150,7 +1148,6 @@
return SUCCESS;
}
/* }}} */
-#endif /* ZEND_MULTIBYTE */
/* {{{ static PHP_INI_MH(OnUpdate_mbstring_substitute_character) */
static PHP_INI_MH(OnUpdate_mbstring_substitute_character)
@@ -1249,9 +1246,7 @@
PHP_INI_ENTRY(mbstring.http_input, pass, PHP_INI_ALL,
OnUpdate_mbstring_http_input)
PHP_INI_ENTRY(mbstring.http_output, pass, PHP_INI_ALL,
OnUpdate_mbstring_http_output)
STD_PHP_INI_ENTRY(mbstring.internal_encoding, NULL, PHP_INI_ALL,
OnUpdate_mbstring_internal_encoding, internal_encoding_name,
zend_mbstring_globals, mbstring_globals)
-#ifdef ZEND_MULTIBYTE
PHP_INI_ENTRY(mbstring.script_encoding, NULL, PHP_INI_ALL,
OnUpdate_mbstring_script_encoding)
-#endif /* ZEND_MULTIBYTE */
PHP_INI_ENTRY(mbstring.substitute_character, NULL, PHP_INI_ALL,
OnUpdate_mbstring_substitute_character)
STD_PHP_INI_ENTRY(mbstring.func_overload, 0,
PHP_INI_SYSTEM, OnUpdateLong, func_overload, zend_mbstring_globals,
mbstring_globals)
@@ -1278,10 +1273,8 @@
mbstring_globals-language =
Re: [PHP-DEV] --enable-zend-multibyte
Derick Rethans wrote: On Tue, 23 Nov 2010, Dmitry Stogov wrote: I've prepared the patch which completely removes compile-time --enable-zend-multibyte configuration directive. The configuration can be changed at run-tume using zend.multibyte=0/1. The patch also fixes Unicode detection for phar archives. I would like to commit it on Thursday. Pending branching on Thursday, perhaps you should commit it tomorrow instead? The patch looks pretty clean! I would do it right now. :) OK, lets wait till tomorrow. At least I didn't hear serious objections about previous patch and several requests to remove --enable-zend-multibyte completely. Thanks. Dmitry. cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] --enable-zend-multibyte
Gustavo Lopes wrote: On Tue, 23 Nov 2010 09:34:49 -, Dmitry Stogov dmi...@zend.com wrote: hi, I've prepared the patch which completely removes compile-time --enable-zend-multibyte configuration directive. The configuration can be changed at run-tume using zend.multibyte=0/1. The patch also fixes Unicode detection for phar archives. I would like to commit it on Thursday. Just two comments: * You have a line commented out with // (line #443) * The phar fix autodetection is wrong. You look for __HALT_COMPILER, but you should be looking for __HALT_COMPILER();, __halt_compiler(); or any casing variation. __HALT_COMPILER, by itself, does nothing (see http://lxr.php.net/opengrok/xref/PHP_TRUNK/Zend/zend_language_parser.y#219 ). See the patches in #53199. Thank you for comments. I'll care about both issues. Thanks. Dmitry. Also remember to close #42396 once you commit a proper fix. Other than that, no objections. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] --enable-zend-multibyte
Hi Chris, I've added a note into UPGRADING, but I didn't change php.ini-* Thanks. Dmitry. Christopher Jones wrote: On 11/23/2010 01:34 AM, Dmitry Stogov wrote: hi, I've prepared the patch which completely removes compile-time --enable-zend-multibyte configuration directive. The configuration can be changed at run-tume using zend.multibyte=0/1. The patch also fixes Unicode detection for phar archives. I would like to commit it on Thursday. Thanks. Dmitry. Hi Dmitry, The UPGRADING file needs to updated. I'd argue that the zend.multibyte should be included in php.ini-* (along with detect_unicode, zend.enable_gc etc) Chris PS a brief RFC capturing the essentials of the change would help the Doc team. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] --enable-zend-multibyte
Hi Christopher, Right now, I'm working on patch to completely remove compile-time --enable-zend-multibyte and replacing it with run-time zend.multibyte. I think the ZEND_MULTIBYTE constant will became useless. I hope I'll post the patch later today. Thanks. Dmitry. Christopher Jones wrote: On 11/19/2010 06:16 AM, Dmitry Stogov wrote: Derick Rethans wrote: On Thu, 18 Nov 2010, Christopher Jones wrote: On 11/18/2010 06:26 AM, Dmitry Stogov wrote: The proposed patch allows compiling PHP with --enable-zend-multibyte and then enable or disable multibyte support at run-time using zend.multibyte=0/1 in php.ini. As result the single binary will be able to support multibyte encodings and run without zend-multibyte overhead dependent on configuration. The patch doesn't affect PHP compiled without --enable-zend-multibyte. I'm going to commit it into trunk before alpha. Any objections? If the overhead of zend.multibyte=0 is negligable, then why not remove remove the configure option altogether? This would reduce the complexity of the implementation and simplify building maintenance. Why have two interdependent ways to get the same outcome if one way will do? Because it has a dependency on mbstring. Actually, it doesn't have compile-time dependencies, so it's probably make sense to remove --enable-zend-multibyte completely. Thanks. Dmitry. Derick Another ease-of-use simplification prior to releasing 5.3.4 is to roll back the addition of ZEND_MULTIBYTE http://svn.php.net/viewvc?view=revisionrevision=305266 In future an ini_get() for zend.multibyte would be the solution for http://bugs.php.net/bug.php?id=52348 Chris -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: break/continue $var
without $var it would be possible to compile break/continue into ZEND_FREE/ZEND_SWITCH_FREE and ZEND_JMP. I also thought it would allow to remove op_array-brk_cont_array completely, but it's also used for exception handling. :( Thanks. Dmitry. Derick Rethans wrote: On Thu, 18 Nov 2010, Dmitry Stogov wrote: Previously we decided to remove break/continue $var syntax. I even implemented it in PHP6 brunch, however it wasn't backported into trunk. Could I do it? Is there a really good reason to remove it? regards, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] --enable-zend-multibyte
CG(multibyte) is defined only inside #ifdef ZEND_MULTIBYTE.
Thanks. Dmitry.
Patrick ALLAERT wrote:
2010/11/18 Dmitry Stogov dmi...@zend.com:
Index: ext/mbstring/mbstring.c
===
--- ext/mbstring/mbstring.c (revision 305494)
+++ ext/mbstring/mbstring.c (working copy)
@@ -1132,6 +1132,9 @@
{
int *list, size;
+ if (!CG(multibyte)) {
+ return FAILURE;
+ }
if (php_mb_parse_encoding_list(new_value, new_value_length, list,
size, 1 TSRMLS_CC)) {
if (MBSTRG(script_encoding_list) != NULL) {
free(MBSTRG(script_encoding_list));
Should this hunk not be inside an #ifdef ZEND_MULTIBYTE ?
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] --enable-zend-multibyte
Derick Rethans wrote: On Thu, 18 Nov 2010, Christopher Jones wrote: On 11/18/2010 06:26 AM, Dmitry Stogov wrote: The proposed patch allows compiling PHP with --enable-zend-multibyte and then enable or disable multibyte support at run-time using zend.multibyte=0/1 in php.ini. As result the single binary will be able to support multibyte encodings and run without zend-multibyte overhead dependent on configuration. The patch doesn't affect PHP compiled without --enable-zend-multibyte. I'm going to commit it into trunk before alpha. Any objections? If the overhead of zend.multibyte=0 is negligable, then why not remove remove the configure option altogether? This would reduce the complexity of the implementation and simplify building maintenance. Why have two interdependent ways to get the same outcome if one way will do? Because it has a dependency on mbstring. Actually, it doesn't have compile-time dependencies, so it's probably make sense to remove --enable-zend-multibyte completely. Thanks. Dmitry. Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] --enable-zend-multibyte
Hi,
The proposed patch allows compiling PHP with --enable-zend-multibyte and
then enable or disable multibyte support at run-time using
zend.multibyte=0/1 in php.ini. As result the single binary will be able
to support multibyte encodings and run without zend-multibyte overhead
dependent on configuration.
The patch doesn't affect PHP compiled without --enable-zend-multibyte.
I'm going to commit it into trunk before alpha.
Any objections?
Thanks. Dmitry.
Index: ext/standard/info.c
===
--- ext/standard/info.c (revision 305494)
+++ ext/standard/info.c (working copy)
@@ -760,7 +760,7 @@
php_info_print_table_row(2, Zend Memory Manager,
is_zend_mm(TSRMLS_C) ? enabled : disabled );
#ifdef ZEND_MULTIBYTE
- php_info_print_table_row(2, Zend Multibyte Support,
enabled);
+ php_info_print_table_row(2, Zend Multibyte Support,
CG(multibyte) ? enabled : disabled);
#else
php_info_print_table_row(2, Zend Multibyte Support,
disabled);
#endif
Index: ext/mbstring/mbstring.c
===
--- ext/mbstring/mbstring.c (revision 305494)
+++ ext/mbstring/mbstring.c (working copy)
@@ -1132,6 +1132,9 @@
{
int *list, size;
+ if (!CG(multibyte)) {
+ return FAILURE;
+ }
if (php_mb_parse_encoding_list(new_value, new_value_length, list,
size, 1 TSRMLS_CC)) {
if (MBSTRG(script_encoding_list) != NULL) {
free(MBSTRG(script_encoding_list));
@@ -1442,8 +1445,10 @@
PHP_RINIT(mb_regex) (INIT_FUNC_ARGS_PASSTHRU);
#endif
#ifdef ZEND_MULTIBYTE
-
zend_multibyte_set_internal_encoding(mbfl_no_encoding2name(MBSTRG(internal_encoding))
TSRMLS_CC);
- php_mb_set_zend_encoding(TSRMLS_C);
+ if (CG(multibyte)) {
+
zend_multibyte_set_internal_encoding(mbfl_no_encoding2name(MBSTRG(internal_encoding))
TSRMLS_CC);
+ php_mb_set_zend_encoding(TSRMLS_C);
+ }
#endif /* ZEND_MULTIBYTE */
return SUCCESS;
@@ -1570,7 +1575,7 @@
MBSTRG(current_internal_encoding) = no_encoding;
#ifdef ZEND_MULTIBYTE
/* TODO: make independent from
mbstring.encoding_translation? */
- if (MBSTRG(encoding_translation)) {
+ if (CG(multibyte) MBSTRG(encoding_translation)) {
zend_multibyte_set_internal_encoding(name
TSRMLS_CC);
}
#endif /* ZEND_MULTIBYTE */
Index: Zend/zend.c
===
--- Zend/zend.c (revision 305494)
+++ Zend/zend.c (working copy)
@@ -93,6 +93,7 @@
ZEND_INI_ENTRY(error_reporting, NULL,
ZEND_INI_ALL, OnUpdateErrorReporting)
STD_ZEND_INI_BOOLEAN(zend.enable_gc, 1,
ZEND_INI_ALL, OnUpdateGCEnabled, gc_enabled,
zend_gc_globals,gc_globals)
#ifdef ZEND_MULTIBYTE
+ STD_ZEND_INI_BOOLEAN(zend.multibyte, 0, ZEND_INI_PERDIR,
OnUpdateBool, multibyte, zend_compiler_globals, compiler_globals)
STD_ZEND_INI_BOOLEAN(detect_unicode, 1, ZEND_INI_ALL, OnUpdateBool,
detect_unicode, zend_compiler_globals, compiler_globals)
#endif
ZEND_INI_END()
Index: Zend/zend_language_scanner.l
===
--- Zend/zend_language_scanner.l(revision 305494)
+++ Zend/zend_language_scanner.l(working copy)
@@ -181,7 +181,7 @@
lex_state-filename = zend_get_compiled_filename(TSRMLS_C);
lex_state-lineno = CG(zend_lineno);
-#ifdef ZEND_MULTIBYTE
+#ifdef ZEND_MULTIBYTE
lex_state-script_org = SCNG(script_org);
lex_state-script_org_size = SCNG(script_org_size);
lex_state-script_filtered = SCNG(script_filtered);
@@ -270,27 +270,32 @@
if (size != -1) {
#ifdef ZEND_MULTIBYTE
- if (zend_multibyte_read_script((unsigned char *)buf, size
TSRMLS_CC) != 0) {
- return FAILURE;
- }
+ if (CG(multibyte)) {
+ if (zend_multibyte_read_script((unsigned char *)buf,
size TSRMLS_CC) != 0) {
+ return FAILURE;
+ }
- SCNG(yy_in) = NULL;
+ SCNG(yy_in) = NULL;
- zend_multibyte_set_filter(NULL TSRMLS_CC);
+ zend_multibyte_set_filter(NULL TSRMLS_CC);
- if (!SCNG(input_filter)) {
- SCNG(script_filtered) = (unsigned
char*)emalloc(SCNG(script_org_size)+1);
- memcpy(SCNG(script_filtered), SCNG(script_org),
SCNG(script_org_size)+1);
- SCNG(script_filtered_size) = SCNG(script_org_size);
+ if
[PHP-DEV] break/continue $var
Hi, Previously we decided to remove break/continue $var syntax. I even implemented it in PHP6 brunch, however it wasn't backported into trunk. Could I do it? Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Adding path_len to all stream functions in trunk
hi, don't we have ext/filter that should check all the dangerous input strings? It would be useless to perform additional checks for constant stings known at compile time (e.g. on include foo.php) Thanks. Dmitry. Rasmus Lerdorf wrote: On 11/15/10 10:12 PM, Stas Malyshev wrote: Hi! Well, it changes the signature of that function, so while we don't break backward binary compatibility, we break forward compatibility within the 5.3 branch. As in, if I change my extension to use this new NoNull string flag, it will no longer work on5.3.3 whereas if I do the if(strlen(filename) != filename_len) check, this will still work in all 5.3 releases. So if you have such extension, and you need to have it compatible with previous versions (e.g. PECL one), use the check. That doesn't prevent us from having the flag in the core code and thus keeping it cleaner. It still worries me a bit. Distros love to separate core extensions into separate packages and if you update one of those without updating the core package, it will break. Hopefully they have hard dependencies so you can't install php-curl-5.3.4 on php-5.3.3, for example. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] Reduce heap memory consumption
Hi,
The attached patch modifies internal ZE data structures.
It gives 4% less heap memory usage on waiting PHP process (2210KB
instead of 2304KB) and up to 7% improvement on applications with many
classes (ZendFramework benchmark uses 17.7M instead of 19M). It also
makes small speedup on some big applications with opcode caches, because
of less memory transfer.
The patch performs the following optimizations:
1) zend_function.pass_rest_by_reference is replaced by
ZEND_ACC_PASS_REST_BY_REFERENCE in zend_function.fn_flags
2) zend_function.return_reference is replaced by
ZEND_ACC_RETURN_REFERENCE in zend_function.fn_flags
3) zend_arg_info.required_num_args removed. it was needed only for
internal functions. Now the first arg_info for internal function (which
has special meaning) is represented by zend_internal_function_info
structure.
4) zend_op_array.size, size_var, size_literal, current_brk_cont,
backpatch_count moved into CG(context), because they are used only
during compilation.
5) zend_op_array.start_op is moved into EG(start_op), because it's used
only for 'interactive' execution of single top-level op-array.
6) zend_op_array.done_pass_two is replaced by ZEND_ACC_DONE_PASS_TWO in
zend_op_array.fn_flags.
7) the sise of op_array.vars is reduced during pass_two.
8) zend_class_entry.constants_updated is replaced by
ZEND_ACC_CONSTANTS_UPDATED in zend_class_entry.ce_flags
9) The size of zend_class_entry is reduced by sharing the same memory
space by different information for internal and user classes. See
zend_class_inttry.info union.
Of course the patch will affect some extensions (e.g. APC and xdebug),
but it shouldn't be difficult to fix them.
I'm going to commit the patch by the end of this week in case of no
objections. I'll also add corresponding notes into UPGRADING.INTERNALS.
Thanks. Dmitry.
Index: ext/soap/soap.c
===
--- ext/soap/soap.c (revision 303306)
+++ ext/soap/soap.c (working copy)
@@ -703,7 +703,6 @@
fe.prototype = NULL;
fe.num_args = 2;
fe.arg_info = NULL;
- fe.pass_rest_by_reference = 0;
INIT_OVERLOADED_CLASS_ENTRY(ce, PHP_SOAP_CLIENT_CLASSNAME,
soap_client_functions,
(zend_function *)fe, NULL, NULL);
Index: ext/pdo/pdo_dbh.c
===
--- ext/pdo/pdo_dbh.c (revision 303306)
+++ ext/pdo/pdo_dbh.c (working copy)
@@ -1308,28 +1308,36 @@
ifunc-function_name = (char*)funcs-fname;
ifunc-scope = dbh-std.ce;
ifunc-prototype = NULL;
+ if (funcs-flags) {
+ ifunc-fn_flags = funcs-flags;
+ } else {
+ ifunc-fn_flags = ZEND_ACC_PUBLIC;
+ }
if (funcs-arg_info) {
+ zend_internal_function_info *info =
(zend_internal_function_info*)funcs-arg_info;
+
ifunc-arg_info = (zend_arg_info*)funcs-arg_info + 1;
ifunc-num_args = funcs-num_args;
- if (funcs-arg_info[0].required_num_args == -1) {
+ if (info-required_num_args == -1) {
ifunc-required_num_args = funcs-num_args;
} else {
- ifunc-required_num_args =
funcs-arg_info[0].required_num_args;
+ ifunc-required_num_args =
info-required_num_args;
}
- ifunc-pass_rest_by_reference =
funcs-arg_info[0].pass_by_reference;
- ifunc-return_reference =
funcs-arg_info[0].return_reference;
+ if (info-pass_rest_by_reference) {
+ if (info-pass_rest_by_reference ==
ZEND_SEND_PREFER_REF) {
+ ifunc-fn_flags |=
ZEND_ACC_PASS_REST_PREFER_REF;
+ } else {
+ ifunc-fn_flags |=
ZEND_ACC_PASS_REST_BY_REFERENCE;
+ }
+ }
+ if (info-return_reference) {
+ ifunc-fn_flags |= ZEND_ACC_RETURN_REFERENCE;
+ }
} else {
ifunc-arg_info = NULL;
ifunc-num_args = 0;
ifunc-required_num_args = 0;
- ifunc-pass_rest_by_reference = 0;
- ifunc-return_reference = 0;
}
- if (funcs-flags) {
- ifunc-fn_flags = funcs-flags;
- } else {
- ifunc-fn_flags = ZEND_ACC_PUBLIC;
- }
namelen = strlen(funcs-fname);
lc_name = emalloc(namelen+1);
Re: [PHP-DEV] Re: PHP Annotations RFC + Patch
Hi Guilherme,
Guilherme Blanco wrote:
Hi Dmitry,
Comments goes inline.
On Wed, Sep 8, 2010 at 10:56 AM, Dmitry Stogov dmi...@zend.com wrote:
Pierrick Charron wrote:
Hi Dmitry,
First thanks for having a look at the patch and taking some time to
give your feedback ! It's much appreciated
2010/9/8 Dmitry Stogov dmi...@zend.com:
Hi Pierrick,
I've taken just a quick look into concept and patch. It looks interesting
and might be useful in some areas, but I see several significant
problems:
1) Have you thought about compatibility with opcode caches? In case I
understood properly, you store annotation as a HashTable of object. But
life-time of objects is restricted by request time, so they won't persist
in
opcode cache and will have to be recreated on each request. It's a huge
overhead. I don't have a good solution for this problem except for using
arrays instead of objects.
You're right annotations are stored in an HashTable. But this
HashTable doesn't store zval objects directly but zend_annotations.
Then it's not a problem. my mistake.
This is the reflection getAnnotation and getAnnotations method which
are in charge of reading the zend_annotation structures and
instantiating the proper annotation object when needed. The zval named
instance in the zend_annotation structure is there only to point to
the annotation instance at runtime so that the state of the annotation
is kept between two calls to the getAnnotation and getAnnotations on a
same code element (I added this directly into the zend_annotation
structure to make it faster but I can imagine a HashTable like you
mentioned earlier where keys are addresses). I'm not yet quite
familiar with the APC source code but since (and correct me if i'm
wrong) APC save zend_class_entry, etc.. in memory we could modify it
to also save the new HashTable *annotations of those structures (like
you already do for doc_comment) excluding the zval *instance.
2) I suppose that usage of annotation would be quite rare case. I don't
think it make sense to extend each op_array, property and class with
additional annotations field. I think it's possible to have a separate
CG(annotaions_map) where the keys might be the addresses of corresponding
classesm, methods and properties.
Annotations are metadata related to a code element. IMHO it make sense
to add them in the appropriate code element structures (even
doc_comment is in those structures) so that APC and other cache will
know that this is something part of the code element and it needs to
be cached and so on.
It's easier to keep it in appropriate places, but it's a waste of memory,
because only a few classes will use it. According to doc comments, in case
we accept annotations, I would represent it as a special kind of annotation.
Separate map(enity - anotations) would require memory only for defined
annotations. Of course it would be a bit more expensive to read it and would
require special handling in opcode hashes.
Argument is not reasonable IMHO.
You may know that most code around web lack of documentation,
specially docblock, so why not separate the docblocks on a separate
structure too?
It would be good to separate it too, as well as different debug
information e.g opline number, filename etc.
Sorry, but your argument is not reasonable.
Each class/property/method should contain all relevant information it
can get, whatever it needs (docblock, annotations, visibility
information, etc). Of course you have a bit of memory overhead, but
it's a pointer, isn't it? So at the end, we will only have overhead
when we have annotations defined.
he pointer takes memory too.
Anyway, I think it's better to make experiment and see how this patch
affects memory consumption. Just load most ZF classes and measure peak
heap usage.
$ USE_ZEND_ALLOC=0 valgrind --tool=massif php zf.php
$ msprintf massif.out.pid
Then we may decide if it make sense to separate annotations or not.
(I can do this test and share results when have time).
Also, decomposing the annotation definition and storage adds a level
of dependency and indirection that may get things harder to comprehend
by newer contributors.
Agree.
3) I think the annotaton syntax must follow PHP syntax as close as
possible
- allow leading \ in QualifiedName
- use '=' in arrays
The actual patch already implement those two points. You can right now
already do
[\My\Own\Annotation(array('foo' = 'bar', 'baz'))]
class FooBar {}
Sorry about that, I just noticed that the EBNF in the RFC was not
updated correctly to reflect those two points. I just made the
modification to replace = by = in arrays and allow the optional
leading \ in the QualidifedName
great.
4) I didn't understand why do we need nested annotations.
Nested annotations could be use for many cases. Doctrine for example
use them a lot to do things like :
[JoinTable(
name=users_phonenumbers,
joinColumns=array(
[JoinColumn(name=user_id, referencedColumnName=id
[PHP-DEV] Re: PHP Annotations RFC + Patch
Hi Pierrick, I've taken just a quick look into concept and patch. It looks interesting and might be useful in some areas, but I see several significant problems: 1) Have you thought about compatibility with opcode caches? In case I understood properly, you store annotation as a HashTable of object. But life-time of objects is restricted by request time, so they won't persist in opcode cache and will have to be recreated on each request. It's a huge overhead. I don't have a good solution for this problem except for using arrays instead of objects. 2) I suppose that usage of annotation would be quite rare case. I don't think it make sense to extend each op_array, property and class with additional annotations field. I think it's possible to have a separate CG(annotaions_map) where the keys might be the addresses of corresponding classesm, methods and properties. 3) I think the annotaton syntax must follow PHP syntax as close as possible - allow leading \ in QualifiedName - use '=' in arrays 4) I didn't understand why do we need nested annotations. The first and second problems are the most important. I don't like to lose performance of existing applications which don't use annotations at all. I think the concept must be redesigned a bit. Also I would like to see an APC patch to check if the implementation works and doesn't lose performance. Thanks. Dmitry. Pierrick Charron wrote: Hi Dmitry, As discussed on IRC, here are the RFC and the patch to add Annotation support in PHP : - RFC : http://wiki.php.net/rfc/annotations - Patch : http://www.adoy.net/php/Annotations.diff All your feedback and suggestions will be more than welcome. Thanks again. Pierrick (or adoy on irc) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Delegate type hint/checks to alternative locations (Re: [PHP-DEV] back to 5.4 alpha)
Hi Derick,
We had a long discussion on RFC and hear you can see the summary of my
opinion:
I think removing the strict type hinting (reverting semantic back to
5.3), but keeping the new syntax and reflection API is a good decision.
However I definitely against of delegation of type-hinting logic into
different location. PHP should be a solid language with single semantic.
This delegation (zend_verify_arg_type callback) allows any extension to
change schematic as it want and this may lead us into situation when
some scripts will work properly only with or without Xdebug (or other
extension). It also restricts possibilities to optimize ZE and introduce
performance lose (now each ZEND_RECV opcode have to call this callback).
Anyway performance is not the main problem because the patch may be
optimized to call callback only if it's defined.
I would suggest to go forward with this decision partially (without
callback and without zval** instead of zval*)
I hope in some point we will come to conclusion about type-hinting...
Thanks. Dmitry.
Derick Rethans wrote:
On Wed, 11 Aug 2010, Derick Rethans wrote:
On Wed, 11 Aug 2010, Stas Malyshev wrote:
So I'd propose doing the following:
1. Moving parameter typing to a feature branch (by branching current trunk and
then rolling back the typing part in the trunk).
2. Starting 5.4 alpha process after that basing on trunk.
Any objections to this?
A little bit; yes. There is indeed 0 consensus for having the strict
typehints. However, instead of removing it altogether, and instead
answering every mail in this thread :P, I wrote/am writing a patch that
removes the hard type checks. It however keeps the parsed structures and
reflection API for it. In this sense, they're actually real hints. The
patch also adds a mechanism similariy to the zend_error_cb mechanism so
that extensions could override the argument type checking. As my use
case for strict checking is development I'd be happy to just move the
hard checks into an extension. I could even offer a soft check. It also
allows some type inference which might be useful for webservice
introspecition generation. I am sure SOAP might have some benefit of
this, and I know that at least pecl/dbus does. The patch is attached,
but not ready (I haven't remove the hard checks yet because things got
busy at work).
I've spend some more time on this, and have attached a new patch that:
- Removes the strict type verification, changing it back into typehints
only.
- Keeps the current syntax so that typehints create structures in the
function entries.
- Keeps the reflection API for the syntax, so that you can query the
typehints.
- Changed the API so that the verification function can also modify the
variables.
I've also written a proof of concept extension at
svn://svn.xdebug.org/svn/php/typed and
http://svn.xdebug.org/cgi-bin/viewvc.cgi/typed/?root=php that implements
both the current strict-type verification, and a form of the option 1
from
http://wiki.php.net/rfc/typecheckingstrictandweak#option_1_current_type_juggeling_rules_with_e_strict_on_data_loss
(I've not done the E_STRICT warnings for all, as the RFC didn't specifiy
when data loss was considered to occur).
Here follows an example script:
?php
function errorHandler( $errno, $string )
{
global $ok;
$ok = $errno;
return true;
}
set_error_handler( 'errorHandler' );
$twelve = 12;
settype( $twelve, 'float' );
$values = array(
true, false,
0, 1, 12, $twelve, 12.23,
'true', 'false',
'0', '1', '12', '12abc', '12.0', '12.34', 'foo',
array(1,2,3), array('345' = 12),
NULL, ''
);
$funcs = array(
'testString', 'testFloat', 'testInt', 'testNumeric',
'testScalar', 'testBool', 'testArray',
);
function testString( string $a ) { }
function testFloat( float $a ) { }
function testInt( int $a ) { }
function testBool( boolean $a ) { }
function testArray( array $a ) { }
function testNumeric( numeric $a ) { }
function testScalar( scalar $a ) { }
echo string float int numeric scalar boolarray\n;
foreach( $values as $value )
{
foreach( $funcs as $func )
{
$ok = true;
$func($value);
echo $ok === true ? pass : ( $ok === 0x1000 ? fail :
warn );
}
echo ' ', str_replace( \n, '', var_export( $value, true ) );
echo \n;
}
?
And the output (with the three different validation/verification methods:
No validation/verification:
der...@kossu:/home/httpd/html/test/verify-arg$ php -dextension=typed.so -dtyped.mode=0 option2.php
[PHP-DEV] Re: PHP Annotations RFC + Patch
Pierrick Charron wrote:
Hi Dmitry,
First thanks for having a look at the patch and taking some time to
give your feedback ! It's much appreciated
2010/9/8 Dmitry Stogov dmi...@zend.com:
Hi Pierrick,
I've taken just a quick look into concept and patch. It looks interesting
and might be useful in some areas, but I see several significant problems:
1) Have you thought about compatibility with opcode caches? In case I
understood properly, you store annotation as a HashTable of object. But
life-time of objects is restricted by request time, so they won't persist in
opcode cache and will have to be recreated on each request. It's a huge
overhead. I don't have a good solution for this problem except for using
arrays instead of objects.
You're right annotations are stored in an HashTable. But this
HashTable doesn't store zval objects directly but zend_annotations.
Then it's not a problem. my mistake.
This is the reflection getAnnotation and getAnnotations method which
are in charge of reading the zend_annotation structures and
instantiating the proper annotation object when needed. The zval named
instance in the zend_annotation structure is there only to point to
the annotation instance at runtime so that the state of the annotation
is kept between two calls to the getAnnotation and getAnnotations on a
same code element (I added this directly into the zend_annotation
structure to make it faster but I can imagine a HashTable like you
mentioned earlier where keys are addresses). I'm not yet quite
familiar with the APC source code but since (and correct me if i'm
wrong) APC save zend_class_entry, etc.. in memory we could modify it
to also save the new HashTable *annotations of those structures (like
you already do for doc_comment) excluding the zval *instance.
2) I suppose that usage of annotation would be quite rare case. I don't
think it make sense to extend each op_array, property and class with
additional annotations field. I think it's possible to have a separate
CG(annotaions_map) where the keys might be the addresses of corresponding
classesm, methods and properties.
Annotations are metadata related to a code element. IMHO it make sense
to add them in the appropriate code element structures (even
doc_comment is in those structures) so that APC and other cache will
know that this is something part of the code element and it needs to
be cached and so on.
It's easier to keep it in appropriate places, but it's a waste of
memory, because only a few classes will use it. According to doc
comments, in case we accept annotations, I would represent it as a
special kind of annotation.
Separate map(enity - anotations) would require memory only for defined
annotations. Of course it would be a bit more expensive to read it and
would require special handling in opcode hashes.
3) I think the annotaton syntax must follow PHP syntax as close as possible
- allow leading \ in QualifiedName
- use '=' in arrays
The actual patch already implement those two points. You can right now
already do
[\My\Own\Annotation(array('foo' = 'bar', 'baz'))]
class FooBar {}
Sorry about that, I just noticed that the EBNF in the RFC was not
updated correctly to reflect those two points. I just made the
modification to replace = by = in arrays and allow the optional
leading \ in the QualidifedName
great.
4) I didn't understand why do we need nested annotations.
Nested annotations could be use for many cases. Doctrine for example
use them a lot to do things like :
[JoinTable(
name=users_phonenumbers,
joinColumns=array(
[JoinColumn(name=user_id, referencedColumnName=id)]
),
inverseJoinColumns=array(
[JoinColumn(name=phonenumber_id, referencedColumnName=id,
unique=true)]
)
)]
The first and second problems are the most important.
I don't like to lose performance of existing applications which don't use
annotations at all. I think the concept must be redesigned a bit.
Right now the only performance decrease is at compile time since the
annotations HashTable are filled by the parser.
I assume parser works with near the same speed without annotations. right?
At runtime you don't
have any performance decrease if you're not using annotations. If you
use them, you'll lose some time when you do getAnnotation() to
instantiate the object, but this instantiation is just done once for
each annotation on a single code element so it make it a little bit
faster if you use them intensively.
I see, but assumptions are not always right, often memory consumption
affects performance directly, it's better to check.
Also I would like to see an APC patch to check if the implementation works
and doesn't lose performance.
I will try to work on an APC patch to store the Annotations HashTable
of every code element structure.
In case you have plain C structures (without objects) it shouldn't be a
big problem. I'm more interested to store annotation separately. In this
case APC support
[PHP-DEV] [PATCH] string_offset access optimization
Hi,
Recently I noticed that reading of string offset is performed in two
steps. At first special string_offset variant of temporary_variable is
created in zend_fetch_dimension_address_read() and then the real string
value is created in _get_zval_ptr_var_string_offset().
I think we can create the real string in the first place. This makes 50%
speed-up on string offset reading operation and allows to eliminate some
checks and conditional brunches in VM.
The patch is attached (don't forget to regenerate zend_vm_execute.h to
test it). However it changes behavior in one bogus case.
The following code now will emit b (currently it generates a fatal
error - cannot use string offset as an array).
$str = abs;
var_dumo($str[1][0]);
I think it's not a problem at all.
b makes sense because abs[1] - b and b[0] - b.
I'm going to commit the patch in case of no objections.
Thanks. Dmitry.
Index: Zend/zend_execute.c
===
--- Zend/zend_execute.c (revision 301262)
+++ Zend/zend_execute.c (working copy)
@@ -177,44 +177,14 @@
return should_free-var = T(var).tmp_var;
}
-static zval *_get_zval_ptr_var_string_offset(zend_uint var, const
temp_variable *Ts, zend_free_op *should_free TSRMLS_DC)
+static zend_always_inline zval *_get_zval_ptr_var(zend_uint var, const
temp_variable *Ts, zend_free_op *should_free TSRMLS_DC)
{
- temp_variable *T = T(var);
- zval *str = T-str_offset.str;
- zval *ptr;
+ zval *ptr = T(var).var.ptr;
- /* string offset */
- ALLOC_ZVAL(ptr);
- T-str_offset.ptr = ptr;
- should_free-var = ptr;
-
- if (T-str_offset.str-type != IS_STRING
- || ((int)T-str_offset.offset 0)
- || (T-str_offset.str-value.str.len =
(int)T-str_offset.offset)) {
- ptr-value.str.val = STR_EMPTY_ALLOC();
- ptr-value.str.len = 0;
- } else {
- ptr-value.str.val = estrndup(str-value.str.val +
T-str_offset.offset, 1);
- ptr-value.str.len = 1;
- }
- PZVAL_UNLOCK_FREE(str);
- Z_SET_REFCOUNT_P(ptr, 1);
- Z_SET_ISREF_P(ptr);
- ptr-type = IS_STRING;
+ PZVAL_UNLOCK(ptr, should_free);
return ptr;
}
-static zend_always_inline zval *_get_zval_ptr_var(zend_uint var, const
temp_variable *Ts, zend_free_op *should_free TSRMLS_DC)
-{
- zval *ptr = T(var).var.ptr;
- if (EXPECTED(ptr != NULL)) {
- PZVAL_UNLOCK(ptr, should_free);
- return ptr;
- } else {
- return _get_zval_ptr_var_string_offset(var, Ts, should_free
TSRMLS_CC);
- }
-}
-
static zend_never_inline zval **_get_zval_cv_lookup(zval ***ptr, zend_uint
var, int type TSRMLS_DC)
{
zend_compiled_variable *cv = CV_DEF_OF(var);
@@ -1286,14 +1256,23 @@
dim = tmp;
}
if (result) {
+ zval *ptr;
+
+ ALLOC_ZVAL(ptr);
+ INIT_PZVAL(ptr);
+ Z_TYPE_P(ptr) = IS_STRING;
+
if (Z_LVAL_P(dim) 0 ||
Z_STRLEN_P(container) = Z_LVAL_P(dim)) {
zend_error(E_NOTICE,
Uninitialized string offset: %ld, Z_LVAL_P(dim));
+ Z_STRVAL_P(ptr) =
STR_EMPTY_ALLOC();
+ Z_STRLEN_P(ptr) = 0;
+ } else {
+ Z_STRVAL_P(ptr) =
(char*)emalloc(2);
+ Z_STRVAL_P(ptr)[0] =
Z_STRVAL_P(container)[Z_LVAL_P(dim)];
+ Z_STRVAL_P(ptr)[1] = 0;
+ Z_STRLEN_P(ptr) = 1;
}
- result-str_offset.str = container;
- PZVAL_LOCK(container);
- result-str_offset.offset =
Z_LVAL_P(dim);
- result-var.ptr_ptr = NULL;
- result-var.ptr = NULL;
+ AI_SET_PTR(result, ptr);
}
return;
}
Index: Zend/micro_bench.php
===
--- Zend/micro_bench.php(revision 301262)
+++ Zend/micro_bench.php(working copy)
@@ -188,6 +188,20 @@
}
}
+function read_hash($n) {
+ $hash = array('test' = 0);
+ for ($i = 0; $i $n; ++$i) {
+ $x = $hash['test'];
+ }
+}
+
+function
Re: [PHP-DEV] Run Time Cache RFC
Hi, s...@geleia.net wrote: On Tue, May 18, 2010 8:22 am, Dmitry Stogov wrote: I'm proposing another optimisation technique implementation for PHP which makes up to 20% speed up on synthetic tests and up to 8% speed up on real-life applications. The technique is similar to inline caches which is very popular in JIT compilers for object oriented languages. http://wiki.php.net/rfc/runtimecache The patch breaks binary and source compatibility but it's not hard to adopt extensions to use it. I'm going to commit the patch on next week in case of no objections. First, I'm sorry I didn't bring this before (as in before the commit), but I missed this thread. As I understand, this patch adds a new place (a regular array) wherein the properties are stored. They can now be in the properties hashtable, in the properties_table array, or in both (but for refcount purposes they're only stored in one place). Dynamic properties require the hash table. Exactly. This feels very hackish. I'm probably missing a crucial point, but why not store everything in properties hash table and then store the zval**'s also in the properties_table if needed. The HashTable construction/destruction takes significant time and more memory, however most operations with declared properties don't require HashTable at all. In case you compare the Zend/micro_bench.php output before and after the patch you'll see a big speedup on ZEND_NEW opcode which is cause by new object structure. The very similar technique is already used in PHP for lazy symbol table initialization. This would avoid having to check whether the hashtable is built and build it if not -- Is there such a significant performance penalty? The API is already clumsy enough and this is bi-storage of properties is something a beginner would find hard to understand (though I understand this will probably not impact the extensions that just use the handlers to access the properties). Of course. The patch only optimizes low-level operations but the API is still the same. Finally, how does this impact debugging? Shouldn't there be a way to disable caching for debugging purposes? Debugger may just check and rebuild properties HashTable if it needs it. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Type hinting
Stas Malyshev wrote: Hi! I see three key options going forward: 1. Implement the table along the lines of what it looks like now, perhaps with minor changes. 2. Implement identical conversion rules to the ones that exist in PHP; That effectively turns type hinting into scalar casting operators (not saying that's a bad thing!) 3. Implement identical conversion rules to the ones that exist in PHP, except for when they really suck. Namely, lose the array-scalar conversions and silent conversions of non-numeric strings to numbers. I'm for 3, Array-scalar must die, as for non-numeric to numbers, I don't have strong opinion as I see no point in it, IMO if it failed, I'd be OK with it. In case we remove 'Array-scalar' and others everywhere, #3 would be the same as #2. I'm for that. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Run Time Cache RFC
Hi Chris, I've added notes for extension maintainers. I hope they will answer all your questions. Thanks. Dmitry. Christopher Jones wrote: Dmitry Stogov wrote: http://wiki.php.net/rfc/runtimecache The patch breaks binary and source compatibility but it's not hard to adopt extensions to use it. Hi Dmitry, Can update the RFC to explain the breakage and It's clear from the patch. The same is explained in RFC in human language, but it's not so exact :) The RFC should give enough explanation so (i) extension maintainers can immediately identify if their extension is going to be affected (ii) readers know where to look in the patch to get more details. If I understand what you implied later in your email, calls to zend_hash_copy should be replaced with calls to object_properties_init. Is this always true? Is this the only extension requirement? What happens if extensions don't do this - will it break or be a performance loss? I must be looking at a different RFC because there's nothing I can see that mentions any of these things. Chris -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Run Time Cache RFC
Hi Jones,
Christopher Jones wrote:
Dmitry Stogov wrote:
Hi,
I'm proposing another optimisation technique implementation for PHP
which makes up to 20% speed up on synthetic tests and up to 8% speed up
on real-life applications.
The technique is similar to inline caches which is very popular in JIT
compilers for object oriented languages.
http://wiki.php.net/rfc/runtimecache
The patch breaks binary and source compatibility but it's not hard to
adopt extensions to use it.
I'm going to commit the patch on next week in case of no objections.
Thanks. Dmitry.
Hi Dmitry,
Can update the RFC to explain the breakage and
It's clear from the patch. The same is explained in RFC in human
language, but it's not so exact :)
Index: Zend/zend.h
===
--- Zend/zend.h (revision 299422)
+++ Zend/zend.h (working copy)
@@ -298,6 +298,7 @@
typedef struct _zend_object {
zend_class_entry *ce;
HashTable *properties;
+ zval **properties_table;
HashTable *guards; /* protects from __get/__set ... recursion */
} zend_object;
@@ -468,11 +469,13 @@
zend_uint ce_flags;
HashTable function_table;
- HashTable default_properties;
HashTable properties_info;
- HashTable default_static_members;
- HashTable *static_members;
+ zval **default_properties_table;
+ zval **default_static_members_table;
+ zval **static_members_table;
HashTable constants_table;
+ int default_properties_count;
+ int default_static_members_count;
const struct _zend_function_entry *builtin_functions;
union _zend_function *constructor;
give the required or
suggested best-practice changes for extensions?
Most affected php extensions already fixed with the same patch.
It usually requires just one line change.
Index: ext/dom/php_dom.c
===
--- ext/dom/php_dom.c (revision 299422)
+++ ext/dom/php_dom.c (working copy)
@@ -1053,7 +1053,6 @@
static dom_object* dom_objects_set_class(zend_class_entry *class_type,
zend_bool hash_copy TSRMLS_DC) /* {{{ */
{
zend_class_entry *base_class;
- zval *tmp;
dom_object *intern;
if (instanceof_function(class_type, dom_xpath_class_entry TSRMLS_CC)) {
@@ -1075,7 +1074,7 @@
zend_object_std_init(intern-std, class_type TSRMLS_CC);
if (hash_copy) {
- zend_hash_copy(intern-std.properties,
class_type-default_properties, (copy_ctor_func_t) zval_add_ref, (void
*) tmp, sizeof(zval *));
+ object_properties_init(intern-std, class_type);
}
return intern;
The complete patch is attached to RFC, and in case anyone likes to
understand it, they have to look into the patch itself.
Thanks. Dmitry.
Thanks,
Chris
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Run Time Cache RFC
Hi, I'm proposing another optimisation technique implementation for PHP which makes up to 20% speed up on synthetic tests and up to 8% speed up on real-life applications. The technique is similar to inline caches which is very popular in JIT compilers for object oriented languages. http://wiki.php.net/rfc/runtimecache The patch breaks binary and source compatibility but it's not hard to adopt extensions to use it. I'm going to commit the patch on next week in case of no objections. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Autoboxing in PHP
Hi Moriyoshi, I took just a quick look through the patch, but for me it looks like a bad idea. Introducing new magic function may bring a lot of troubles and open a new door for exploit writer (we already have problems with __toString() method). Also I afraid, this magic method will make php slower even if scripts don't use this future (at least the patch disables code specialization for ZEND_INIT_METHOD_CALL) and make some future type propagation optimizations non-applicable. At last the patch introduces 18 new grammar conflicts and I think it's not acceptable. Thanks. Dmitry. Moriyoshi Koizumi wrote: Hey, Just to let you know about a new RFC for adding autoboxing feature in PHP. Look at http://wiki.php.net/rfc/autoboxing . Regards, Moriyoshi -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/trunk/ ext/bz2/bz2.dsp ext/ctype/ctype.dsp ext/curl/curl.dsp ext/dba/dba.dsp ext/dom/dom.dsp ext/exif/exif.dsp ext/ext_skel_win32.php ext/gd/gd.dsp ext/gettex
Hi Rob, May be I missed the discussion... I'm not sure if it's a good idea to have all this scarp in SVN. Personally, I won't support these DSP files just because I don't use Visual Studio 6.0 to build php. This method was deprecated for ages. I'm also not sure if next php version is going to support VC 6.0 at all. Thanks. Dmitry. Rob Richards wrote: rrichardsWed, 28 Apr 2010 14:41:51 + Revision: http://svn.php.net/viewvc?view=revisionrevision=298702 Log: revert change #298288: Remove old dsp/dsw/makefile files Bug: http://bugs.php.net/298288 (error getting bug information) Changed paths: A + php/php-src/trunk/ext/bz2/bz2.dsp (from php/php-src/trunk/ext/bz2/bz2.dsp:r298287) A + php/php-src/trunk/ext/ctype/ctype.dsp (from php/php-src/trunk/ext/ctype/ctype.dsp:r298287) A + php/php-src/trunk/ext/curl/curl.dsp (from php/php-src/trunk/ext/curl/curl.dsp:r298287) A + php/php-src/trunk/ext/dba/dba.dsp (from php/php-src/trunk/ext/dba/dba.dsp:r298287) A + php/php-src/trunk/ext/dom/dom.dsp (from php/php-src/trunk/ext/dom/dom.dsp:r298287) A + php/php-src/trunk/ext/exif/exif.dsp (from php/php-src/trunk/ext/exif/exif.dsp:r298287) U php/php-src/trunk/ext/ext_skel_win32.php A + php/php-src/trunk/ext/gd/gd.dsp (from php/php-src/trunk/ext/gd/gd.dsp:r298287) A + php/php-src/trunk/ext/gettext/gettext.dsp (from php/php-src/trunk/ext/gettext/gettext.dsp:r298287) A + php/php-src/trunk/ext/iconv/iconv.dsp (from php/php-src/trunk/ext/iconv/iconv.dsp:r298287) A + php/php-src/trunk/ext/imap/imap.dsp (from php/php-src/trunk/ext/imap/imap.dsp:r298287) A + php/php-src/trunk/ext/interbase/interbase.dsp (from php/php-src/trunk/ext/interbase/interbase.dsp:r298287) A + php/php-src/trunk/ext/json/json.dsp (from php/php-src/trunk/ext/json/json.dsp:r298287) A + php/php-src/trunk/ext/ldap/ldap.dsp (from php/php-src/trunk/ext/ldap/ldap.dsp:r298287) A + php/php-src/trunk/ext/mbstring/mbstring.dsp (from php/php-src/trunk/ext/mbstring/mbstring.dsp:r298287) A + php/php-src/trunk/ext/mcrypt/mcrypt.dsp (from php/php-src/trunk/ext/mcrypt/mcrypt.dsp:r298287) A + php/php-src/trunk/ext/mssql/mssql.dsp (from php/php-src/trunk/ext/mssql/mssql.dsp:r298287) A + php/php-src/trunk/ext/mysql/mysql.dsp (from php/php-src/trunk/ext/mysql/mysql.dsp:r298287) A + php/php-src/trunk/ext/mysql/mysql.mak (from php/php-src/trunk/ext/mysql/mysql.mak:r298287) A + php/php-src/trunk/ext/mysqli/mysqli.dsp (from php/php-src/trunk/ext/mysqli/mysqli.dsp:r298287) A + php/php-src/trunk/ext/oci8/oci8.dsp (from php/php-src/trunk/ext/oci8/oci8.dsp:r298287) A + php/php-src/trunk/ext/openssl/openssl.dsp (from php/php-src/trunk/ext/openssl/openssl.dsp:r298287) A + php/php-src/trunk/ext/openssl/openssl.mak (from php/php-src/trunk/ext/openssl/openssl.mak:r298287) A + php/php-src/trunk/ext/pgsql/pgsql.dsp (from php/php-src/trunk/ext/pgsql/pgsql.dsp:r298287) A + php/php-src/trunk/ext/pgsql/pgsql.mak (from php/php-src/trunk/ext/pgsql/pgsql.mak:r298287) A + php/php-src/trunk/ext/pspell/pspell.dsp (from php/php-src/trunk/ext/pspell/pspell.dsp:r298287) A + php/php-src/trunk/ext/shmop/shmop.dsp (from php/php-src/trunk/ext/shmop/shmop.dsp:r298287) A + php/php-src/trunk/ext/simplexml/simplexml.dsp (from php/php-src/trunk/ext/simplexml/simplexml.dsp:r298287) A + php/php-src/trunk/ext/skeleton/skeleton.dsp (from php/php-src/trunk/ext/skeleton/skeleton.dsp:r298287) A + php/php-src/trunk/ext/snmp/snmp.dsp (from php/php-src/trunk/ext/snmp/snmp.dsp:r298287) A + php/php-src/trunk/ext/soap/php_soap.dsp (from php/php-src/trunk/ext/soap/php_soap.dsp:r298287) A + php/php-src/trunk/ext/sockets/sockets.dsp (from php/php-src/trunk/ext/sockets/sockets.dsp:r298287) A + php/php-src/trunk/ext/sqlite/sqlite.dsp (from php/php-src/trunk/ext/sqlite/sqlite.dsp:r298287) A + php/php-src/trunk/ext/sybase_ct/sybase_ct.dsp (from php/php-src/trunk/ext/sybase_ct/sybase_ct.dsp:r298287) A + php/php-src/trunk/ext/tidy/tidy.dsp (from php/php-src/trunk/ext/tidy/tidy.dsp:r298287) A + php/php-src/trunk/ext/tokenizer/tokenizer.dsp (from php/php-src/trunk/ext/tokenizer/tokenizer.dsp:r298287) A + php/php-src/trunk/ext/xml/xml.mak (from php/php-src/trunk/ext/xml/xml.mak:r298287) A + php/php-src/trunk/ext/xmlreader/xmlreader.dsp (from php/php-src/trunk/ext/xmlreader/xmlreader.dsp:r298287) A + php/php-src/trunk/ext/xmlrpc/xmlrpc.dsp (from php/php-src/trunk/ext/xmlrpc/xmlrpc.dsp:r298287) A + php/php-src/trunk/ext/xmlwriter/xmlwriter.dsp (from php/php-src/trunk/ext/xmlwriter/xmlwriter.dsp:r298287) A +
[PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/trunk/ ext/bz2/bz2.dsp ext/ctype/ctype.dsp ext/curl/curl.dsp ext/dba/dba.dsp ext/dom/dom.dsp ext/exif/exif.dsp ext/ext_skel_win32.php ext/gd/gd.dsp ext/gettex
Hi Rob, The problem is not to have them. The problem is to keep them in consistency in case someone adds/removes a file. I like the idea of automatic generation much more. Thanks. Dmitry. Rob Richards wrote: Hi Dmitry, The discussion was a while ago (Steph was working on a way to autogenerate them at the time). Unfortunately it failed miserably (buildconf error'd out) when I tried to use it (tried it after the dsps had been removed). Until that is working (or some other mechanism is in place to generate them), I see no reason to remove them. They easily convert to newer VS versions so not specifically for VC6. The project files are nice to have around for simple organization/access in VS especially since we already have them, so its not so much about using them to make a build as it is for the VS integration. Rob Hi Rob, May be I missed the discussion... I'm not sure if it's a good idea to have all this scarp in SVN. Personally, I won't support these DSP files just because I don't use Visual Studio 6.0 to build php. This method was deprecated for ages. I'm also not sure if next php version is going to support VC 6.0 at all. Thanks. Dmitry. Rob Richards wrote: rrichardsWed, 28 Apr 2010 14:41:51 + Revision: http://svn.php.net/viewvc?view=revisionrevision=298702 Log: revert change #298288: Remove old dsp/dsw/makefile files Bug: http://bugs.php.net/298288 (error getting bug information) Changed paths: A + php/php-src/trunk/ext/bz2/bz2.dsp (from php/php-src/trunk/ext/bz2/bz2.dsp:r298287) A + php/php-src/trunk/ext/ctype/ctype.dsp (from php/php-src/trunk/ext/ctype/ctype.dsp:r298287) A + php/php-src/trunk/ext/curl/curl.dsp (from php/php-src/trunk/ext/curl/curl.dsp:r298287) A + php/php-src/trunk/ext/dba/dba.dsp (from php/php-src/trunk/ext/dba/dba.dsp:r298287) A + php/php-src/trunk/ext/dom/dom.dsp (from php/php-src/trunk/ext/dom/dom.dsp:r298287) A + php/php-src/trunk/ext/exif/exif.dsp (from php/php-src/trunk/ext/exif/exif.dsp:r298287) U php/php-src/trunk/ext/ext_skel_win32.php A + php/php-src/trunk/ext/gd/gd.dsp (from php/php-src/trunk/ext/gd/gd.dsp:r298287) A + php/php-src/trunk/ext/gettext/gettext.dsp (from php/php-src/trunk/ext/gettext/gettext.dsp:r298287) A + php/php-src/trunk/ext/iconv/iconv.dsp (from php/php-src/trunk/ext/iconv/iconv.dsp:r298287) A + php/php-src/trunk/ext/imap/imap.dsp (from php/php-src/trunk/ext/imap/imap.dsp:r298287) A + php/php-src/trunk/ext/interbase/interbase.dsp (from php/php-src/trunk/ext/interbase/interbase.dsp:r298287) A + php/php-src/trunk/ext/json/json.dsp (from php/php-src/trunk/ext/json/json.dsp:r298287) A + php/php-src/trunk/ext/ldap/ldap.dsp (from php/php-src/trunk/ext/ldap/ldap.dsp:r298287) A + php/php-src/trunk/ext/mbstring/mbstring.dsp (from php/php-src/trunk/ext/mbstring/mbstring.dsp:r298287) A + php/php-src/trunk/ext/mcrypt/mcrypt.dsp (from php/php-src/trunk/ext/mcrypt/mcrypt.dsp:r298287) A + php/php-src/trunk/ext/mssql/mssql.dsp (from php/php-src/trunk/ext/mssql/mssql.dsp:r298287) A + php/php-src/trunk/ext/mysql/mysql.dsp (from php/php-src/trunk/ext/mysql/mysql.dsp:r298287) A + php/php-src/trunk/ext/mysql/mysql.mak (from php/php-src/trunk/ext/mysql/mysql.mak:r298287) A + php/php-src/trunk/ext/mysqli/mysqli.dsp (from php/php-src/trunk/ext/mysqli/mysqli.dsp:r298287) A + php/php-src/trunk/ext/oci8/oci8.dsp (from php/php-src/trunk/ext/oci8/oci8.dsp:r298287) A + php/php-src/trunk/ext/openssl/openssl.dsp (from php/php-src/trunk/ext/openssl/openssl.dsp:r298287) A + php/php-src/trunk/ext/openssl/openssl.mak (from php/php-src/trunk/ext/openssl/openssl.mak:r298287) A + php/php-src/trunk/ext/pgsql/pgsql.dsp (from php/php-src/trunk/ext/pgsql/pgsql.dsp:r298287) A + php/php-src/trunk/ext/pgsql/pgsql.mak (from php/php-src/trunk/ext/pgsql/pgsql.mak:r298287) A + php/php-src/trunk/ext/pspell/pspell.dsp (from php/php-src/trunk/ext/pspell/pspell.dsp:r298287) A + php/php-src/trunk/ext/shmop/shmop.dsp (from php/php-src/trunk/ext/shmop/shmop.dsp:r298287) A + php/php-src/trunk/ext/simplexml/simplexml.dsp (from php/php-src/trunk/ext/simplexml/simplexml.dsp:r298287) A + php/php-src/trunk/ext/skeleton/skeleton.dsp (from php/php-src/trunk/ext/skeleton/skeleton.dsp:r298287) A + php/php-src/trunk/ext/snmp/snmp.dsp (from php/php-src/trunk/ext/snmp/snmp.dsp:r298287) A + php/php-src/trunk/ext/soap/php_soap.dsp (from php/php-src/trunk/ext/soap/php_soap.dsp:r298287) A + php/php-src/trunk/ext/sockets/sockets.dsp (from php/php-src/trunk/ext/sockets/sockets.dsp:r298287) A + php/php-src/trunk/ext/sqlite/sqlite.dsp (from
Re: [PHP-DEV] [RFC] Performance improvements
Hi Christopher, Christopher Jones wrote: Dmitry Stogov wrote: Hi, I've added the corresponding patch for APC, so anyone is able to verify the performance difference. http://wiki.php.net/rfc/performanceimprovements Dmitry, What do you think caused the fw difference in absolute figures between the APC and Zend Optimizer+ tests? The fw is a synthetic benchmark which loads all the Zend Framework classes (more than 100 files with classes). It looks like we optimized our cache for this situation little bit better. Sorry, but this time I'm not able to disclose Optimizer+ internals. However, I did only a quick look over APC code and didn't try to profile it. May be it does something wrong (performs the same work several times). Thanks. Dmitry. Chris -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Performance improvements
Hi Derick,
Thank you for review.
Derick Rethans wrote:
On Tue, 13 Apr 2010, Dmitry Stogov wrote:
I've published all the patches, their description and performance evaluation
at http://wiki.php.net/rfc/performanceimprovements
I've a few questions and comments:
http://wiki.php.net/rfc/performanceimprovements#zend_engine_vm_tuning
You're writing that the ZEND_CATCH opcode has to be used with a constant
class name, so how would catch ( self $e ) be represented in op-codes
now?
I dropped such possibility. Does someone use it?
For me it looks useless. If it's really necessary I can try not to drop it.
Some patches seem to be changing tests; I think we should avoid that and
instead create a new test that only runs for the new version, and add a
qualifier to the old one (the one that you're now modifying) to *not*
run with the new PHP version.
It's not a problem. I can do it.
The patch made one test to emit warning messages in different order, but
on the other hand the change made the performance better.
2-empty_hash.diff.txt seems to be ignoring some of our coding
guidelines:
if(ht-nTableMask) pefree(ht-arBuckets, ht-persistent);
should clearly be:
if (ht-nTableMask) {
pefree(ht-arBuckets, ht-persistent);
}
Could you please check the space between if and (, and always
using braces for all the patches?
I'll fix it.
3-literals.diff.txt makes the opcode arrays again a bit more complex to
disect. Perhaps you could add a bit of documentation describing how all
the things work together now? Like which index in whivh fields points to
which other structure etc?
it's not a problem to add a few words about it:
- each op_array has an array of literals (constant values)
- opocde operands don't contain zval directly any more but points to
this table instead
- during compilation they are accessible by index e.g.
op_array-literals[opline-op1.constant].constant
- the pass_two() changes indexes into pointers so during execution they
are accessible by
opline-op1.zv
Do you like me to add this into WiKi or some text file?
4-interned.diff.txt
The change in array.c, is that strictly necessary, or just another
performance improvement?
It's necessary. The change disables in-place string modification.
Things like make me wonder, whether it would work to make this macro
part of efree() ? (or create an esfree() (string-free) instead?
- efree(Z_STRVAL_P(value));
+ if (!IS_INTERNED(Z_STRVAL_P(value))) {
+ efree(Z_STRVAL_P(value));
+ }
It makes sense :)
In the RFC you write Note: The usage of special data structure (e.g.
zend_string) instead of char* throughout PHP would probably be a more
consistent solution, but it would be a huge break. We are going to look
into this solution in the future.. This would be an API break only,
right, very similar to what the Unicode patch did with zstr's?
I don't understand the *only*. Of course it would be an API break which
would need to be propagated over the whole PHP code. It has some
similarity with zstr, but it would be more powerful structure which
contain value, length, hash_value, interned flag, may be refcount. It
also may be extended with charset/encoding to implement Unicode strings
in a Parrot way.
Thanks. Dmirty.
with kind regards,
Derick
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: php 5.3, memory leak
Hi Alexey, I'll try to take care of it. It's definitely a bug, however I'm not sure how to fix it yet. (I've updated the bug report). Thanks. Dmitry. Alexey Zakhlestin wrote: I was doing some tests with long-running scripts recently and noticed that 5.3 is leaking memory in cases where 5.2 wasn't. Looks like there is bug report already: http://bugs.php.net/bug.php?id=48781 PHP 5.3 was announced as more suitable for long-running scripts than 5.2 was, because of circular-references garbage collector. But this bug undermines this. So, I believe this bug is a critical one. Should be a blocker for 5.3.3 Bug-report tells that this bug was assigned to Dmitry, but, as there were not comments from him, I am not even sure if he's actually aware of the problem. Did anyone try to investigate the issue? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Performance improvements
Hi,
I've added the corresponding patch for APC, so anyone is able to verify
the performance difference.
http://wiki.php.net/rfc/performanceimprovements
Thanks. Dmitry.
Dmitry Stogov wrote:
Hi,
I've published all the patches, their description and performance
evaluation at http://wiki.php.net/rfc/performanceimprovements
In two words the patches give 0-20% improvement even on real-life
applications.
I'm going to commit them into trunk in a week in case of no objections.
Of course, they are binary incompatible. Some extensions (especially VM
depended e.g. APC, xdebug, etc) will have to be modified to support the
changes.
Thanks. Dmitry.
Zeev Suraski wrote:
Hi,
Over the last few weeks we've been working on several ideas we had for
performance enhancements. We've managed to make some good progress.
Our initial tests show roughly 10% speed improvement on real world
apps. On pure OO code we're seeing as much as 25% improvement (!)
While this still is a work in progress (and not production quality
code yet) we want to get feedback sooner rather than later. The diff
(available at http://bit.ly/aDPTmv) applies cleanly to trunk. We'd be
happy for people to try it out and send comments.
What does it contain?
1) Constant operands have been moved from being embedded within the
opcodes into a separate literal table. In additional to the zval it
contains pre-calculated hash values for string literals. As result PHP
uses less memory and doesn't have to recalculate hash values for
constants at run-time.
2) Lazy HashTable buckets allocation – we now only allocate the
buckets array when we actually insert data into the hash for the first
time. This saves both memory and time as many hash tables do not have
any data in them.
3) Interned strings (see
http://en.wikipedia.org/wiki/String_interninghttp://en.wikipedia.org/wiki/String_interning).
Most strings known at compile-time are allocated in a single copy with
some additional information (pre-calculated hash value, etc.). We try
to make most incarnations of a given string point to that same single
version, allowing us to save memory, but more importantly - run
comparisons by comparing pointers instead of comparing strings and
avoid redundant hash value calculations.
A couple of notes:
a. Not all of the strings are interned - which means that if a
pointer comparison fails, we still go through a string comparison;
But if it succeeds - it's good enough.
b. We'd need to add support for this in the bytecode caches. We'd be
happy to work with the various bytecode cache teams to guide how to
implement support so that you do not have to intern on each request.
To get a better feel for what interning actually does, consider the
following examples:
// Lookup for $arr will not calculate a hash value, and will only
require a pointer comparison in most cases
// Lookup for foo in $arr will not calculate a hash value, and will
only require a pointer comparison
// The string foo will not have to be allocated as a key in the Bucket
// blah when assigned doesn't have to be duplicated
$arr[“foo”] = “blah”;
$a = “b”;
if ($a == “b”) { // pointer comparison only
...
}
Comments welcome!
Zeev
Patch available at: http://bit.ly/aDPTmv
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Performance improvements
Kalle Sommer Nielsen wrote: Hi Dmitry 2010/4/19 Dmitry Stogov dmi...@zend.com: Hi, I've added the corresponding patch for APC, so anyone is able to verify the performance difference. http://wiki.php.net/rfc/performanceimprovements Thanks. Dmitry. Dmitry Stogov wrote: In the patched config.w32, you wrote zend_string.c, not apc_string.c which is wrong ;-) thanks :) Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC] Performance improvements
Hi,
I've published all the patches, their description and performance
evaluation at http://wiki.php.net/rfc/performanceimprovements
In two words the patches give 0-20% improvement even on real-life
applications.
I'm going to commit them into trunk in a week in case of no objections.
Of course, they are binary incompatible. Some extensions (especially VM
depended e.g. APC, xdebug, etc) will have to be modified to support the
changes.
Thanks. Dmitry.
Zeev Suraski wrote:
Hi,
Over the last few weeks we've been working on several ideas we had for
performance enhancements. We've managed to make some good progress. Our
initial tests show roughly 10% speed improvement on real world apps. On
pure OO code we're seeing as much as 25% improvement (!)
While this still is a work in progress (and not production quality code
yet) we want to get feedback sooner rather than later. The diff
(available at http://bit.ly/aDPTmv) applies cleanly to trunk. We'd be
happy for people to try it out and send comments.
What does it contain?
1) Constant operands have been moved from being embedded within the
opcodes into a separate literal table. In additional to the zval it
contains pre-calculated hash values for string literals. As result PHP
uses less memory and doesn't have to recalculate hash values for
constants at run-time.
2) Lazy HashTable buckets allocation – we now only allocate the buckets
array when we actually insert data into the hash for the first time.
This saves both memory and time as many hash tables do not have any data
in them.
3) Interned strings (see
http://en.wikipedia.org/wiki/String_interninghttp://en.wikipedia.org/wiki/String_interning).
Most strings known at compile-time are allocated in a single copy with
some additional information (pre-calculated hash value, etc.). We try
to make most incarnations of a given string point to that same single
version, allowing us to save memory, but more importantly - run
comparisons by comparing pointers instead of comparing strings and avoid
redundant hash value calculations.
A couple of notes:
a. Not all of the strings are interned - which means that if a pointer
comparison fails, we still go through a string comparison; But if it
succeeds - it's good enough.
b. We'd need to add support for this in the bytecode caches. We'd be
happy to work with the various bytecode cache teams to guide how to
implement support so that you do not have to intern on each request.
To get a better feel for what interning actually does, consider the
following examples:
// Lookup for $arr will not calculate a hash value, and will only
require a pointer comparison in most cases
// Lookup for foo in $arr will not calculate a hash value, and will
only require a pointer comparison
// The string foo will not have to be allocated as a key in the Bucket
// blah when assigned doesn't have to be duplicated
$arr[“foo”] = “blah”;
$a = “b”;
if ($a == “b”) { // pointer comparison only
...
}
Comments welcome!
Zeev
Patch available at: http://bit.ly/aDPTmv
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Performance improvements
Hi Richard,
Richard Quadling wrote:
On 13 April 2010 14:53, Dmitry Stogov dmi...@zend.com wrote:
Hi,
I've published all the patches, their description and performance evaluation
at http://wiki.php.net/rfc/performanceimprovements
In two words the patches give 0-20% improvement even on real-life
applications.
I'm going to commit them into trunk in a week in case of no objections.
Of course, they are binary incompatible. Some extensions (especially VM
depended e.g. APC, xdebug, etc) will have to be modified to support the
changes.
Thanks. Dmitry.
Zeev Suraski wrote:
Hi,
Over the last few weeks we've been working on several ideas we had for
performance enhancements. We've managed to make some good progress. Our
initial tests show roughly 10% speed improvement on real world apps. On
pure OO code we're seeing as much as 25% improvement (!)
While this still is a work in progress (and not production quality code
yet) we want to get feedback sooner rather than later. The diff (available
at http://bit.ly/aDPTmv) applies cleanly to trunk. We'd be happy for people
to try it out and send comments.
What does it contain?
1) Constant operands have been moved from being embedded within the
opcodes into a separate literal table. In additional to the zval it contains
pre-calculated hash values for string literals. As result PHP uses less
memory and doesn't have to recalculate hash values for constants at
run-time.
2) Lazy HashTable buckets allocation – we now only allocate the buckets
array when we actually insert data into the hash for the first time. This
saves both memory and time as many hash tables do not have any data in them.
3) Interned strings (see
http://en.wikipedia.org/wiki/String_interninghttp://en.wikipedia.org/wiki/String_interning).
Most strings known at compile-time are allocated in a single copy with
some additional information (pre-calculated hash value, etc.). We try to
make most incarnations of a given string point to that same single version,
allowing us to save memory, but more importantly - run comparisons by
comparing pointers instead of comparing strings and avoid redundant hash
value calculations.
A couple of notes:
a. Not all of the strings are interned - which means that if a pointer
comparison fails, we still go through a string comparison; But if it
succeeds - it's good enough.
b. We'd need to add support for this in the bytecode caches. We'd be
happy to work with the various bytecode cache teams to guide how to
implement support so that you do not have to intern on each request.
To get a better feel for what interning actually does, consider the
following examples:
// Lookup for $arr will not calculate a hash value, and will only require
a pointer comparison in most cases
// Lookup for foo in $arr will not calculate a hash value, and will only
require a pointer comparison
// The string foo will not have to be allocated as a key in the Bucket
// blah when assigned doesn't have to be duplicated
$arr[“foo”] = “blah”;
$a = “b”;
if ($a == “b”) { // pointer comparison only
...
}
Comments welcome!
Zeev
Patch available at: http://bit.ly/aDPTmv
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Firstly, I'm a total novice in terms of C code.
My comment relates to the following line in the wiki ...
On the other hand it'll have to check if the HashTable was
initialized on each new element insertion.
So, in PHP code, I assume it looks something like ...
?php
$Hash = null;
function addToHash($Hash, $Value) {
// Is the hash ready?
if (is_null($Hash)) {
$Hash = array();
}
// Add the value.
$Hash[] = $Value;
}
addToHash($Hash, 'First');
addToHash($Hash, 'Second');
?
Each time a value is added, the hash is checked to see if it is ready
to receive the value.
That makes sense.
Rather than testing to see if the hash is ready, is it
possible/appropriate/feasible to have something like this ...
A function reference/pointer which points to function2 at compile time
- I suspect each hash would need this.
function1 which adds to the hash.
function2 which will initialize the hash, call function1to add the
value and then replace the reference to function2 to function1.
That way, there is no additional testing. Essentially the start point
is a wrapped function1 which gets unwrapped on the first use.
Thereafter the unwrapped version is always used.
I use this sort of thing in PHP with closures and in JavaScript. The
common use is to run some code based upon a value (say like a switch
statement). Once known, running the switch statement each time is
redundant.
Is something like this feasible?
As I already answered in different email on most moder CPUs indirect
call is more expensive than comparison and conditional jump. Even in PHP
indirect call must be slower than if (isset($arr)) {} else {}
Thanks. Dmitry.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net
Re: [PHP-DEV] [RFC] Performance improvements
Hi Jared,
Jared Williams wrote:
Hi
Wondering if there is any plans to for new serialization
method?
Not in this patch.
One of the things that igbinary does is store strings only
once, and now that the engine supports string interning natively,
seems that serialization and deserialization could benefit.
Though I guess igbinary could be patched to take advantage of
string interning?
Probably it can. However, I've never looked into it, so I can't be sure.
Thanks. Dmitry.
-Jared
-Original Message-
From: Dmitry Stogov [mailto:dmi...@zend.com]
Sent: 13 April 2010 14:53
To: internals@lists.php.net
Subject: [PHP-DEV] [RFC] Performance improvements
Hi,
I've published all the patches, their description and
performance evaluation at
http://wiki.php.net/rfc/performanceimprovements
In two words the patches give 0-20% improvement even on
real-life applications.
I'm going to commit them into trunk in a week in case of no
objections.
Of course, they are binary incompatible. Some extensions
(especially VM depended e.g. APC, xdebug, etc) will have to
be modified to support the changes.
Thanks. Dmitry.
Zeev Suraski wrote:
Hi,
Over the last few weeks we've been working on several ideas
we had for
performance enhancements. We've managed to make some good
progress.
Our initial tests show roughly 10% speed improvement on real world
apps. On pure OO code we're seeing as much as 25% improvement (!)
While this still is a work in progress (and not production quality
code
yet) we want to get feedback sooner rather than later. The diff
(available at http://bit.ly/aDPTmv) applies cleanly to
trunk. We'd be
happy for people to try it out and send comments.
What does it contain?
1) Constant operands have been moved from being embedded within
the
opcodes into a separate literal table. In additional to the zval
it
contains pre-calculated hash values for string literals. As
result PHP
uses less memory and doesn't have to recalculate hash values for
constants at run-time.
2) Lazy HashTable buckets allocation - we now only allocate the
buckets array when we actually insert data into the hash
for the first time.
This saves both memory and time as many hash tables do not have
any
data in them.
3) Interned strings (see
http://en.wikipedia.org/wiki/String_interninghttp://en.wikip
edia.org/wiki/String_interning).
Most strings known at compile-time are allocated in a
single copy with
some additional information (pre-calculated hash value,
etc.). We try
to make most incarnations of a given string point to that
same single
version, allowing us to save memory, but more importantly - run
comparisons by comparing pointers instead of comparing strings and
avoid redundant hash value calculations.
A couple of notes:
a. Not all of the strings are interned - which means that if a
pointer comparison fails, we still go through a string comparison;
But if it succeeds - it's good enough.
b. We'd need to add support for this in the bytecode
caches. We'd be
happy to work with the various bytecode cache teams to guide how
to
implement support so that you do not have to intern on each
request.
To get a better feel for what interning actually does, consider
the
following examples:
// Lookup for $arr will not calculate a hash value, and will only
require a pointer comparison in most cases // Lookup for
foo in $arr
will not calculate a hash value, and will only require a pointer
comparison // The string foo will not have to be
allocated as a key
in the Bucket // blah when assigned doesn't have to be
duplicated
$arr[foo] = blah;
$a = b;
if ($a == b) { // pointer comparison only
...
}
Comments welcome!
Zeev
Patch available at: http://bit.ly/aDPTmv
--
PHP Internals - PHP Runtime Development Mailing List To
unsubscribe, visit: http://www.php.net/unsub.php
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Performance improvements
Hi Kalle, Kalle Sommer Nielsen wrote: Hi 2010/4/13 Dmitry Stogov dmi...@zend.com: Hi, I've published all the patches, their description and performance evaluation at http://wiki.php.net/rfc/performanceimprovements The only thing I notice while browsing the patches was that you need to add the zend_string.c and other new files to win32/build/config.w32 so that it gets built on Windows aswell. Thanks. I'll update the patches. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Performance improvements
Hi Moriyoshi,
Moriyoshi Koizumi wrote:
Hi,
Wouldn't it suffice to add a field for the hash value and a flag that
indicates its validity to zval instead of appending zend_literal
everywhere?
We used the approach you suggest on the early stages of development, but
then realized that passing zend_literal* adds more power with the same
cost. Think about classes and method names, which need to be converted
to lower case. Especially for them we could pass original name and
zend_literal* which represent lower-cased name.
Thanks. Dmitry.
Moriyoshi
On Wed, Mar 24, 2010 at 11:12 PM, Zeev Suraski z...@zend.com wrote:
Hi,
Over the last few weeks we've been working on several ideas we had for
performance enhancements. We've managed to make some good progress. Our
initial tests show roughly 10% speed improvement on real world apps. On
pure OO code we're seeing as much as 25% improvement (!)
While this still is a work in progress (and not production quality code yet)
we want to get feedback sooner rather than later. The diff (available at
http://bit.ly/aDPTmv) applies cleanly to trunk. We'd be happy for people to
try it out and send comments.
What does it contain?
1) Constant operands have been moved from being embedded within the opcodes
into a separate literal table. In additional to the zval it contains
pre-calculated hash values for string literals. As result PHP uses less
memory and doesn't have to recalculate hash values for constants at
run-time.
2) Lazy HashTable buckets allocation – we now only allocate the buckets
array when we actually insert data into the hash for the first time. This
saves both memory and time as many hash tables do not have any data in them.
3) Interned strings (see
http://en.wikipedia.org/wiki/String_interninghttp://en.wikipedia.org/wiki/String_interning).
Most strings known at compile-time are allocated in a single copy with some
additional information (pre-calculated hash value, etc.). We try to make
most incarnations of a given string point to that same single version,
allowing us to save memory, but more importantly - run comparisons by
comparing pointers instead of comparing strings and avoid redundant hash
value calculations.
A couple of notes:
a. Not all of the strings are interned - which means that if a pointer
comparison fails, we still go through a string comparison; But if it
succeeds - it's good enough.
b. We'd need to add support for this in the bytecode caches. We'd be happy
to work with the various bytecode cache teams to guide how to implement
support so that you do not have to intern on each request.
To get a better feel for what interning actually does, consider the
following examples:
// Lookup for $arr will not calculate a hash value, and will only require a
pointer comparison in most cases
// Lookup for foo in $arr will not calculate a hash value, and will only
require a pointer comparison
// The string foo will not have to be allocated as a key in the Bucket
// blah when assigned doesn't have to be duplicated
$arr[“foo”] = “blah”;
$a = “b”;
if ($a == “b”) { // pointer comparison only
...
}
Comments welcome!
Zeev
Patch available at: http://bit.ly/aDPTmv
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Performance improvements
Moriyoshi Koizumi wrote: Hi, On Thu, Mar 25, 2010 at 8:41 AM, Stanislav Malyshev s...@zend.com wrote: Hi! Wouldn't it suffice to add a field for the hash value and a flag that indicates its validity to zval instead of appending zend_literal everywhere? Enlarging zval would be costly (the engine uses tons of zvals) and may also be more complicated to track (all zval operations now would also have to take care to set the flag right - what if we forget in some extension to set it right?). I think it's better not to mess with zval. If all the constants were interned, then we should not need zend_literal in the first place because we can store the hash values separately in an array whose indices correspond to those of the interned string vector. In general you are right, but having separate hash_value for constant operands allow us to specialize executor in more efficient way. Also, the interned strings buffer may be overflown (or disabled) and in this case we will have regular strings for constants. Plus, I think the hash value can be stored in the following extra bytes of the string buffer pointed by str.val. Do you mean interned or regular strings? For interned strings we already do near the same as you suggest. Thanks. Dmitry. Moriyoshi -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Performance improvements
Hi Rasmus,
Rasmus Lerdorf wrote:
On 03/24/2010 07:12 AM, Zeev Suraski wrote:
Hi,
Over the last few weeks we've been working on several ideas we had for
performance enhancements. We've managed to make some good progress. Our
initial tests show roughly 10% speed improvement on real world apps. On
pure OO code we're seeing as much as 25% improvement (!)
While this still is a work in progress (and not production quality code
yet) we want to get feedback sooner rather than later. The diff
(available at http://bit.ly/aDPTmv) applies cleanly to trunk. We'd be
happy for people to try it out and send comments.
What does it contain?
1) Constant operands have been moved from being embedded within the
opcodes into a separate literal table. In additional to the zval it
contains pre-calculated hash values for string literals. As result PHP
uses less memory and doesn't have to recalculate hash values for
constants at run-time.
2) Lazy HashTable buckets allocation – we now only allocate the buckets
array when we actually insert data into the hash for the first time.
This saves both memory and time as many hash tables do not have any data
in them.
3) Interned strings (see
http://en.wikipedia.org/wiki/String_interninghttp://en.wikipedia.org/wiki/String_interning).
Most strings known at compile-time are allocated in a single copy with
some additional information (pre-calculated hash value, etc.). We try
to make most incarnations of a given string point to that same single
version, allowing us to save memory, but more importantly - run
comparisons by comparing pointers instead of comparing strings and avoid
redundant hash value calculations.
A couple of notes:
a. Not all of the strings are interned - which means that if a pointer
comparison fails, we still go through a string comparison; But if it
succeeds - it's good enough.
b. We'd need to add support for this in the bytecode caches. We'd be
happy to work with the various bytecode cache teams to guide how to
implement support so that you do not have to intern on each request.
To get a better feel for what interning actually does, consider the
following examples:
// Lookup for $arr will not calculate a hash value, and will only
require a pointer comparison in most cases
// Lookup for foo in $arr will not calculate a hash value, and will
only require a pointer comparison
// The string foo will not have to be allocated as a key in the Bucket
// blah when assigned doesn't have to be duplicated
$arr[“foo”] = “blah”;
$a = “b”;
if ($a == “b”) { // pointer comparison only
...
}
Comments welcome!
The changes to ext/standard/array.c in that patch don't seem to have
anything to do with the rest of the patch. You should probably separate
that and just commit that part to trunk now.
You are wrong. The modifications in array.c prevent in-place string
modification, because it can be an interned string or even an interned
string from opcode cache shared across several PHP processes.
As far as I can tell the only pecl extensions broken by this patch are:
phk, apc, bcompiler, and automap. Which isn't too bad. But yes, I
could definitely use some help in figuring out how to add support for
this in pecl/apc.
I think the support for literal tables is clear and can be implemented
without problems. APC support for interned strings would be a bit tricky
so I'm going to describe the idea (or even provide a patch) in few days.
Thanks. Dmitry.
-Rasmus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] PHP_5_3 GC segfaults
Hi Rasmus,
Let me know how to reproduce them and I'll try to look into them.
Thanks. Dmitry.
Rasmus Lerdorf wrote:
I'm seeing some GC-related segfaults in current PHP_5_3. I haven't had
time to dive into it very far. All I have is a couple of bts and the
request that triggers it, but it is a gallery2 request and there is a
lot of code there. I'll see if I can get it down to something
manageable. The first bt is:
Program received signal SIGSEGV, Segmentation fault.
0x7f4d6b3df8f1 in gc_zval_possible_root (zv=0x232e098) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_gc.c:143
143 GC_ZOBJ_CHECK_POSSIBLE_ROOT(zv);
(gdb) bt
#0 0x7f4d6b3df8f1 in gc_zval_possible_root (zv=0x232e098) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_gc.c:143
#1 0x7f4d6b3ce11b in zend_hash_destroy (ht=0x2323e78) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_hash.c:526
#2 0x7f4d6b3c14ff in _zval_dtor_func (zvalue=0x232df78) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_variables.c:43
#3 0x7f4d6b3b5ccd in _zval_dtor (zval_ptr=0x232df58) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_variables.h:35
#4 _zval_ptr_dtor (zval_ptr=0x232df58) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_execute_API.c:435
#5 0x7f4d6b3ce11b in zend_hash_destroy (ht=0x2323f88) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_hash.c:526
#6 0x7f4d6b3c14ff in _zval_dtor_func (zvalue=0x232df28) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_variables.c:43
#7 0x7f4d6b3b5ccd in _zval_dtor (zval_ptr=0x23561e8) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_variables.h:35
#8 _zval_ptr_dtor (zval_ptr=0x23561e8) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_execute_API.c:435
#9 0x7f4d6b3ce11b in zend_hash_destroy (ht=0x2323ce0) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_hash.c:526
#10 0x7f4d6b3e0e69 in zend_object_std_dtor (object=0x2355790) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_objects.c:45
#11 0x7f4d6b3e0e89 in zend_objects_free_object_storage
(object=0x232e098) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_objects.c:114
#12 0x7f4d6b3e47c9 in zend_objects_store_del_ref_by_handle_ex
(handle=9, handlers=value optimized out)
at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_objects_API.c:220
#13 0x7f4d6b3e47e3 in zend_objects_store_del_ref (zobject=0x2342c00)
at /home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_objects_API.c:172
#14 0x7f4d6b3b5ccd in _zval_dtor (zval_ptr=0x22fe8b8) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_variables.h:35
#15 _zval_ptr_dtor (zval_ptr=0x22fe8b8) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_execute_API.c:435
#16 0x7f4d6b3ce11b in zend_hash_destroy (ht=0x2323bb0) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_hash.c:526
#17 0x7f4d6b3e0e69 in zend_object_std_dtor (object=0x22fe990) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_objects.c:45
#18 0x7f4d6b3e0e89 in zend_objects_free_object_storage
(object=0x232e098) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_objects.c:114
#19 0x7f4d6b3e42fc in zend_objects_store_free_object_storage
(objects=0x7f4d6bb79f58) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_objects_API.c:92
#20 0x7f4d6b3b82e5 in shutdown_executor () at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_execute_API.c:298
#21 0x7f4d6b3c21d2 in zend_deactivate () at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend.c:890
#22 0x7f4d6b36e182 in php_request_shutdown (dummy=value optimized
out) at /home/rasmus/src/php/php-src/branches/PHP_5_3/main/main.c:1606
And another:
Program received signal SIGSEGV, Segmentation fault.
zval_mark_grey (pz=0x114f458) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_gc.c:356
356 p = Z_ARRVAL_P(pz)-pListHead;
(gdb) bt
#0 zval_mark_grey (pz=0x114f458) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_gc.c:356
#1 0x7f7ef6d57e39 in zval_mark_grey (pz=0x114f458) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_gc.c:367
#2 0x7f7ef6d5846d in gc_mark_roots () at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_gc.c:417
#3 gc_collect_cycles () at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend_gc.c:628
#4 0x7f7ef6d3b2a5 in zend_deactivate () at
/home/rasmus/src/php/php-src/branches/PHP_5_3/Zend/zend.c:900
#5 0x7f7ef6ce7182 in php_request_shutdown (dummy=value optimized
out) at /home/rasmus/src/php/php-src/branches/PHP_5_3/main/main.c:1606
#6 0x7f7ef6dc4f83 in php_apache_request_dtor (r=0xee3148) at
/home/rasmus/src/php/php-src/branches/PHP_5_3/sapi/apache2handler/sapi_apache2.c:493
(gdb) p pz
$1 = (zval *) 0x114f458
(gdb) p *pz
$2 = {value = {lval = 0, dval = 0, str = {val = 0x0, len = 17070608}, ht
= 0x0,
[PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_2/Zend/zend_object_handlers.c branches/PHP_5_3/NEWS branches/PHP_5_3/main/fopen_wrappers.c branches/PHP_5_3/sapi/cgi/cgi_main.c trunk/main/fop
Jani Taskinen wrote: On 09/07/2009 09:25 AM, Dmitry Stogov wrote: Hi, I'm wondered why the shebang line support was removed from scanner. Because it didn't make any sense for other SAPIs. You could ask Ilia more about it: - r273178 | iliaa | 2009-01-09 19:21:12 +0200 (Fri, 09 Jan 2009) | 4 lines MFH: Corrected fix for bug #46844 to only trigger on the 1st line of CLI opened files. I think it's better to be consistent between different SAPI. I would prefer all of them always skip shebang line instead of checking it only in CGI and CLI. Anyway it doesn't cost the performance degradation we got. Note that going back to this solution requires extra open() and fstat() syscalls in case the requested file already stored in opcode cache. It might be a visible slowdown for FastCGI sapi. Any ideas how to avoid it without adding this thing into the scanner? SAPI knows nothing about opcode caches, so it's not possible to do in general way. Thanks. Dmitry. --Jani -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: SOAP_MARSHAL_DATETIME (or: bug #44383)
Hi David, sorry for delay. The patch can't be used as it breaks ext/soap build as shared extension. timelib function are compiled into php and aren't exported :( May be you will be able to change the patch using ext/date public API. Also we will need a patch for PHP6 to commit it into both branches, but it's the second priority. Thanks. Dmitry. David Zülke wrote: On 28.07.2009, at 19:14, Jani Taskinen wrote: Dmitry Stogov wrote: David Zülke wrote: On 28.07.2009, at 13:32, Dmitry Stogov wrote: Hi David, I took only a quick look, but I like the patch. In case it doesn't break any tests, it should be committed at least into HEAD. I agree to commit it into 5.3 too, but RMs take the final decision. The only thing I didn't understood - why win32/php_stdint.h is needed. Ah, yes, that's probably an oversight. Good catch. The headers were copy-pasted from some ext/date file :) Another thing that just occured to me is that we now have a dependency on ext/date; I think I had trouble compiling ext/soap as a standalone extension like this. Must check again. Any hints? I think ext/date can't be removed or compiled as shared extension. If it's the case, the only problem may be with unexported symbols. Just try to compile/test it as shared extension on Linux and Windows. ext/date is not an extension, it's part of core, just like ext/standard so you shouldn't have any problems with it. Attached (and also posted here: http://pastie.org/575559 and here: http://bugs.php.net/44383) is an update of the patch. It has the following updates: 1) all tests adjusted, reviewed and changed to work in sequence (only one of the several variants was not commented out before) 2) fixed handling of unix timestamp longs There are the following outstanding issues (I'm going on a holiday tomorrow, so I figured I'd shoot you an update in the meantime): 1) there seem to be problems with negative timezone offsets (see tests ext/soap/tests/schema/schema0(89|91).phpt) unless I'm confused 2) at the minute, it breaks ext/soap/tests/schema/schema064.phpt. Except for one of the cases where the formatting is indeed wrong, it's due to time zones not being included. That obviously needs fixing, but I'd like to point out that the current tests do not seem to be in compliance with the XML Schema specification, which for gMonthDay, gDay and gMonth states: An optional following time zone qualifier is allowed as for date. The rule for date and timezone postfixes is For timezoned values, append the canonical representation of the ·recoverable timezone·. A recoverable timezone is an offset value in the format [+-]hh:mm. http://www.w3.org/TR/xmlschema-2/ has all the details, but maybe I'm missing something. For gYearMonth and gYear, I think the dateTime timezone rules apply, which means the canonical representation must always be in UTC (and thus carry Z as the timezone identifier, which is what the tests currently expect, so those are correct). Finally, when I compile as an extension, I get the following error once I reach tests that execute code relying on timelib: dyld: lazy symbol binding failed: Symbol not found: _timelib_time_ctor Referenced from: .../extensions/no-debug-non-zts-20090626/soap.so Expected in: flat namespace dyld: Symbol not found: _timelib_time_ctor Referenced from: .../extensions/no-debug-non-zts-20090626/soap.so Expected in: flat namespace I compiled without --enable-soap, then used the correct phpize in ext/soap, ran configure with the correct --with-php-config=.../blah/php-config, make install and enabled the extension in php.ini. Feedback/thoughts/advice appreciated. I'll be back in ~2 weeks. Cheers, - David -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: SOAP_MARSHAL_DATETIME (or: bug #44383)
Hi Derick, Derick Rethans wrote: On Tue, 11 Aug 2009, Dmitry Stogov wrote: The patch can't be used as it breaks ext/soap build as shared extension. timelib function are compiled into php and aren't exported :( May be you will be able to change the patch using ext/date public API. We can also just export them... Which symbols are they? Probably it make sense for the future release. php6? Thanks. Dmitry. regards, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: SOAP_MARSHAL_DATETIME (or: bug #44383)
Hi David, I took only a quick look, but I like the patch. In case it doesn't break any tests, it should be committed at least into HEAD. I agree to commit it into 5.3 too, but RMs take the final decision. The only thing I didn't understood - why win32/php_stdint.h is needed. Thanks. Dmitry. David Zülke wrote: *bump* Dmitry, did you have a chance to review this patch yet? - David On 22.06.2009, at 15:21, David Zülke wrote: Hi folks, attached is a patch (with the respective test cases) that implements DateTime marshalling from and to xsd:dateTime in ext/soap as requested in http://bugs.php.net/44383 Right now, it is implemented for xsd:date, xsd:time and xsd:dateTime, but not for other types defined in W3C XML Schema such as gDayMonth; I don't really think it makes sense mapping from and to DateTimes in this case (from DateTime to gDayMonth would work, but the other way round would prove rather difficult). Some notes about this patch: - it conforms strictly to the XML Schema specification by only producing canonical representations of values when generating xsd:dateTime and xsd:time values. Specifically: - it will not generate trailing zeroes on microseconds (in other words, it simply generates a fractional second part as mandated by the specification), but it will accept such values - UTC is always used as the timezone (one of the tests in ext/date/tests that mirrors SBR1-echoDate from http://www.w3.org/TR/2007/REC-soap12-testcollection-20070427/#SBR1-echoDate currently does this wrong), but it will accept any timezone - xsd:time produces current date when generating a DateTime object - xsd:date is relatively straightfoward as well: - produces 00:00:00 as the time when creating a DateTime object - accepts any time when parsing - also supports timezones - as a side effect of the patch, microseconds are now supported in time values (for xsd:time and xsd:dateTime), hence the removed comment in to_xml_time The tests have several permutations, but all but one is commented out each. The test_schema() function does some odd (but understandable) stunts with output buffering and global variables that make it impossible to test more than one case at a time. We didn't want to produce a million test files for the several variants; is there a nicer way to test this properly? This feature is enabled by a SoapClient feature called SOAP_MARSHAL_DATETIME. I think this is a reasonable choice. Greetings, David schema089.phptsoap_marshal_datatype.diff.txtschema087.phptschema088.phptschema086.phptschema090.phptschema091.phpt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: SOAP_MARSHAL_DATETIME (or: bug #44383)
David Zülke wrote: On 28.07.2009, at 13:32, Dmitry Stogov wrote: Hi David, I took only a quick look, but I like the patch. In case it doesn't break any tests, it should be committed at least into HEAD. I agree to commit it into 5.3 too, but RMs take the final decision. The only thing I didn't understood - why win32/php_stdint.h is needed. Ah, yes, that's probably an oversight. Good catch. The headers were copy-pasted from some ext/date file :) Another thing that just occured to me is that we now have a dependency on ext/date; I think I had trouble compiling ext/soap as a standalone extension like this. Must check again. Any hints? I think ext/date can't be removed or compiled as shared extension. If it's the case, the only problem may be with unexported symbols. Just try to compile/test it as shared extension on Linux and Windows. Thanks. Dmitry. - David -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Fwd: [SOAP] SOAPClient authentication problem
Hi David, Please report a bug on bugs.php.net (assign it to dmitry). I'll look into it later. Thanks. Dmitry. David Zülke wrote: This sounds like a serious issue, but I'm not sure if it's in libxml or in ext/soap. Will have a look later; but maybe Dmitry or someone else knows off the top of their heads? - David Begin forwarded message: From: Davide Romanini davide.roman...@gmail.com Date: 30. Juni 2009 11:49:30 MESZ To: s...@lists.php.net Subject: [SOAP] SOAPClient authentication problem Reply-To: d.roman...@cineca.it Hi, Today I found a nasty problem with a simple php SOAP client. Never had problems before, but today I have the following error at SOAPClient constructor line: SoapClient::SoapClient(http://www.w3.org/2001/xml.xsd): failed to open stream: HTTP request failed! HTTP/1.1 401 Authorization Required The source is as simple as: $client = new SoapClient(http://my.host.com/my_web_service?wsdl;, array( 'trace' = TRUE, 'login'='mylogin', 'password'='secret' ) ); It seems that the php xml parser tries to fetch the url http://www.w3.org/2001/xml.xsd at wsdl parsing time. Sniffing the network operations I found that php uses my login and password (for the web service) also to access external references! :-O GET /2001/xml.xsd HTTP/1.0 Authorization: Basic bXlsb2dpbjpzZWNyZXQ= Host: www.w3.org In the past probably w3.org just ignored the issue, but now I receive an HTTP 401 Unauthorized error in response... In any case it is a serious security issue if SOAPClient sends password around the web, when the intent is that they are used only for the web service host! I tried the following PHP versions: PHP 5.2.3-1ubuntu6.5 (cli) (built: Feb 11 2009 19:55:53) Copyright (c) 1997-2007 The PHP Group Zend Engine v2.2.0, Copyright (c) 1998-2007 Zend Technologies PHP 5.2.8 (cli) (built: Dec 17 2008 00:54:27) Copyright (c) 1997-2008 The PHP Group Zend Engine v2.2.0, Copyright (c) 1998-2008 Zend Technologies with Zend Extension Manager v1.0.11, Copyright (c) 2003-2006, by Zend Technologies with Zend Optimizer v3.2.0, Copyright (c) 1998-2006, by Zend Technologies with Zend Debugger v5.2.2, Copyright (c) 1999-2006, by Zend Technologies Regards, Davide -- PHP Soap Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Soap over SSL and
it's not the only reason. ext/soap also supports digital authentication which is still unsupported by streams. (may be something else). Thanks. Dmitry. David Zülke wrote: On 10.07.2009, at 17:00, Hannes Magnusson wrote: On Fri, Jul 10, 2009 at 16:58, Uwe Schindlertheta...@php.net wrote: As far as I know, SOAP does not use the HTTP wrappers directly, it uses only sockets/ssl for communication (so the context applies only to the lower level SSL socket). So CURL is not used, because PHP's HTTP streams are not used. A. You are right. But that was because PHP streams didn't support chunked encoding, wasn't it? Dmitry: There is no reason why SOAP should be doing its own magic anylonger? Yes, that was the reason. http://article.gmane.org/gmane.comp.php.devel/53763 - David -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Type hinting/casting request for vote
1) The patch introduce several new reserved words (resource, numeric, scalar, object). This may break existing applications which use these names as function or class names. 2) The patch should define something like ZEND_ARG_TYPE_INFO() 3) do we really need IS_CLASS constant? 4) arg_info-array_type_hint should be probably changed into arg_info-type_hint. May be it's better to use bitset mask for it. Then we won't need IS_SCALAR and IS_NUMERIC as well and arg_info-allow_null field. Otherwise the patch looks good, but note that for now it only slowdowns execution a bit because of addition checks. I hope, it'll be possible to use these hints in the future to generate more optimal code. +1 for php6 after improvements Thanks. Dmitry. Ilia Alshanetsky wrote: Last week or so there was a fairly detailed discussion on the internals list regarding type hinting based on my original patch. Since then the patch has been revised to address the major concerns that were identified (breakage of binary compatibility) as well extended with additional functionality (object hint, type casting, reflection support, etc...). The final patch is available here: http://ilia.ws/patch/type_hint_53_v2.txt The test suit is available here: http://ia.gd/patch/type_hint_tests.tar.bz2 I would like to ask all developers to voice their opinions of whether it makes sense to add this to 5.3 or to throw it away (either one is fine btw). To keep the process simple flamewar free, please restrict yourself to +/- (1/0), next week monday I'll run a tally of the votes and based on the result we can determine how to proceed further. Ilia -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Method call improvements
Hi Guilherme, 5.3 is closed for major updates (it is in RC state). I would try to look into this when we develop a strategy for next PHP version. Thanks. Dmitry. Guilherme Blanco wrote: Hi guys, What's the status on this one?! It's an important optimization that should be considered. Save more than a million method calls on a framework does not worth? None gave a final word on this subject. I could not see this commited in 5.3 neither in HEAD. So...can someone notify me about the status of this??? Cheers, On Thu, Jan 22, 2009 at 10:20 AM, Dmitry Stogov dmi...@zend.com wrote: Marcus Boerger wrote: Aren't we able to bind these at least partially to the function call opcode, in case we know they are constant? If all is constsnt we could even store the whole lookup in the opcode. Well you'd have to convince Zend to do that because os far they have always been against this approach. We can't modify opcode it self as it'll break opcode caches. However we can introduce some indirect table associated with op_array, which can be used to implement inline caches without direct opcode modification (in the same way as IS_CV variables work). There are a lot of papers about polymorphic inline caches (e.g. http://research.sun.com/self/papers/pics.html) which we probably should use to not to invite bicycle. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Method call improvements
Hi Paul, Paul Biggar wrote: On Mon, May 11, 2009 at 7:47 PM, Guilherme Blanco guilhermebla...@gmail.com wrote: What's the status on this one?! I think it died from neglect. But it was a really good idea. One question that was raised was: On Thu, Jan 22, 2009 at 10:20 AM, Dmitry Stogov dmi...@zend.com wrote: However we can introduce some indirect table associated with op_array, which can be used to implement inline caches without direct opcode modification (in the same way as IS_CV variables work). There are a lot of papers about polymorphic inline caches (e.g. http://research.sun.com/self/papers/pics.html) which we probably should use to not to invite bicycle. You can't actually use PICs or even ICs with the Zend engine, because you can't insert code into the callee method's header (you would need a JIT). You also wouldn't want to, since PHP can't use the recompilation techniques that Self had. You can use lookup caches, which is exactly what the original patch was. I know PHP limitations, and I meant additional lookup caches for one or few results connected directly to ZEND_INIT_METHOD_CALL (and family) opcodes. FWIW, since PHP has a static inheritence chain, the best approach seems to be to build a virtual dispatch table, instead of a hashtable for functions. However, there might be some esoteric extensions which make this difficult. The real things is even worse as during compilation of a class it's parent class doesn't have to be known. So construction of VMTs becomes a bit problematic. BTW we could think in this way... Thanks. Dmitry. Paul -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Method call improvements
Paul Biggar wrote: Hi Stas, Dmitry, On Tue, May 12, 2009 at 2:01 AM, Stanislav Malyshev s...@zend.com wrote: IHMO it's not static enough. I.e., since PHP is not compiled, we can not create VD table for the class until runtime inheritance, which means that the code using this class can use method resolution more efficient than name-function, i.e. hashtable. These lookups can be cached (i.e. CV style) but I don't see how they can be altogether prevented. On Tue, May 12, 2009 at 7:25 AM, Dmitry Stogov dmi...@zend.com wrote: The real things is even worse as during compilation of a class it's parent class doesn't have to be known. So construction of VMTs becomes a bit problematic. BTW we could think in this way... Apologies, I'm not familiar with run-time inheritence in PHP. My understanding was that when a classes source code is compiled, its parent classes must be known. When is this not the case? The parent class may be defined in other file that is loaded at runtime using include() statement. It's very usual case. So the PHP first loads the include file and then declares child class at runtime. Must it be known for the class' first instantiation? Of course. :) In the worst case, it _might_ be cheaper to build it at instantiation time, but I would have to look up how expensive that is in a more static language to be sure. Certainly, it is currently so expensive that almost anything else would be better (including the OP's patch). I don't see how run-time VMT contraction may help, because calls to virtual method must know VMT offset at compile-time. Thanks. Dmitry. Thanks, Paul -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Method call improvements
Paul Biggar wrote: On Tue, May 12, 2009 at 9:43 AM, Dmitry Stogov dmi...@zend.com wrote: Apologies, I'm not familiar with run-time inheritence in PHP. My understanding was that when a classes source code is compiled, its parent classes must be known. When is this not the case? The parent class may be defined in other file that is loaded at runtime using include() statement. It's very usual case. So the PHP first loads the include file and then declares child class at runtime. Must it be known for the class' first instantiation? Of course. :) On Tue, May 12, 2009 at 7:25 AM, Dmitry Stogov dmi...@zend.com wrote: The real things is even worse as during compilation of a class it's parent class doesn't have to be known. So construction of VMTs becomes a bit problematic. BTW we could think in this way... OK, so I dont understand this exactly. Is it correct to say that if a class uses inheritance its compilation will be deferred until its first instantiation? Or is it compiled when it is seen, and its parent backpatched in later. When is later? The classes which parent isn't known during compilation inherited at run-time by DECLARE_INHERITED_CLASS opcode. It patches property and method tablas, checks for method compatibility, etc But I think its fair to say that it has static inheritance - that is, its full inheritance chain is known before it can be instantiated, and it can never be changed after that. Right, but it has a lot of dynamic issues anyway. E.g. parent class may be changed or loaded from different file. Thanks. Dmitry. In the worst case, it _might_ be cheaper to build it at instantiation time, but I would have to look up how expensive that is in a more static language to be sure. Certainly, it is currently so expensive that almost anything else would be better (including the OP's patch). I don't see how run-time VMT contraction may help, because calls to virtual method must know VMT offset at compile-time. Right. Construction is fine. Their use is not. I don't know what I was thinking. So it looks like the best way forwards is still the OP's patch? Thanks, Paul -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Scanner diet with fixes, etc.
Hi Matt, I wasn't able to look into all details of the patch, but in general I like it, as it fixes bugs and makes scanner smaller. I think you can commit it. Although this patch doesn't fix the EOF handling related to mmap(). Thanks. Dmitry. Matt Wilmas wrote: Hi guys, - Original Message - From: Nuno Lopes Sent: Thursday, April 30, 2009 The patch looks generally ok. However I'll need a few more days to review it carefully and throughly. (you can merge it in the meantime if you want). I'm just slighty concern with the amount of parsing we are now doing by hand, and with the possible (local) security bugs we might be introducing.. Am I understanding this properly, that this addresses the re2c EOF bug? So we have an RC planned for next week (freeze Monday evening). Can you get this fixed and released by then as Marcus is unable to do this himself? So this addresses some of the re2c EOF problems, but I don't know if it addresses all of them or not. I haven't had the time yet for a full review. Anyway, Matt can surelly comment on this. Yes, it addresses the re2c EOF issues for strings and comments, as they were the problem ones that allowed NULL bytes, and scanned past the EOF NULL. As I said to Dmitry, I'm not sure if it's now possible to remove the temporary mmap() fixes that he wanted removed before the next RC (??), or if there would still be problems with re2c scanning other tokens, even though they can't contain NULLs. I didn't attempt to make any changes there, since I'm not familiar with what's been done. I just wanted to finally send the patch for others to review, and decide what to do, so I won't commit any changes yet in the meantime. :-) Nuno - Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Scanner diet with fixes, etc.
Hi Matt, Does this patch fix EOF handling issues related to mmap()? (e.g. parsing of files with size 4096, 8192, ...). Now we have two dirty fixes to handle them correctly. The patch is quite big to understand it quickly. I'll probably take a look on weekend. -ANY_CHAR [^\x00] +ANY_CHAR [^] Is [^] a correct regular expression? Thanks. Dmitry. Matt Wilmas wrote: Hi Dmitry, Brian, all, Here's a scanner patch that I mentioned awhile ago, with a possible way to work around the re2c EOF handling issues. The primary change is to do a manual scan like I talked about in areas that match large amounts and can contain NULL bytes (strings/comments, which are now scanned faster too), as is done for inline HTML. I called it a diet :-) because it removes my complicated string regex patterns from a couple years ago, which doesn't make the .l file much smaller after adding the manual scan code (easier to understand...?), but it does result in a ~34k reduction of 5.3's generated .c file... This fixes Bug #46817, as well as a better, more proper fix for the older Bug #42767, both related to ending comments. Now inline HTML chunks aren't broken up when a tag starting with s is encountered (script for JS, span, etc.), since it's unlikely to be a long PHP script tag. If an opening PHP SCRIPT tag was used with a capital S, it was missed if it wasn't the first thing scanned: var_dump(token_get_all(HTML... SCRIPT language=php)); Single-line comments with a Windows newline didn't include the full \r\n: var_dump(token_get_all(?php // Comment\r\n?)); Finally, part of the optimized scanning is that, for double quoted strings, when the first variable is encountered (making it non-constant), the amount that's been scanned up to that point is remembered, which can then be skipped over (up to the variable) after returning the quote token. Previously that initial part of the string was rescanned -- the cost dependent on how far into the string the first var is. I think that's about all -- I'll send another message if I forgot to mention anything... Just wanted to send this along quick for to you guys to look at or whatever. It was basically done last week, I just had to do a couple finishing touches and verify that everything was OK. http://realplain.com/php/scanner_diet.diff (Merged changes, but didn't test yet.) http://realplain.com/php/scanner_diet_5_3.diff Thanks, Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [PATCH] double to long conversion change
I looked into it some time ago. Really, I don't understand what it's going to fix (the need of preserving low bits looks strange for me). Also it breaks ~30 tests. Thanks. Dmitry. Johannes Schlüter wrote: On Fri, 2009-04-10 at 14:45 -0500, Matt Wilmas wrote: Hi Dmitry, I finally updated the patches: http://realplain.com/php/dval_to_lval.diff http://realplain.com/php/dval_to_lval_5_3.diff Has anybody (Dmitry?) reviewed this or other feedback? johannes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Support for Transfer-Encoding: chunked in http stream wrapper
Hi,
Thanks for comments.
The updated patch fixes RFC incompatibilities. It just ignores
extensions and trailer.
The patch also exports chunked filter into user space so now it can be
used with any streams (see test in the patch).
In case user opens http stream and server responds with
Transfer-Encoding: chunked header, php applies chunked filter
automatically and doesn't bypass Transfer-Encoding header to user.
The automatic decoding may be disabled using stream_context.
stream_context_create(array(http=array(auto_decode=0)));
Any objections against applying it into 5.3?
I think it's very easy to add automatic decompression of HTTP responses
with HTTP headers Content-Encoding: gzip (and others) using similar
patch (except for filters are already implemented and the patch will
need to update only http_fopen_wrapper.c).
Thanks. Dmitry.
Sara Golemon wrote:
The attached patch implements automatic decoding of chunked
transfer-encoding.
Any objections against committing the patch into PHP_5_3?
I didn't have objections when I offered this filter several years ago,
and I still don't. I do recall Andi (or perhaps it was someone else)
saying it was adding unnecessary complexity to the http wrapper and
such things should be left for cURL though, and this is why it didn't
go into 5.1
Maybe someone has ideas about patch improvements?
I'd add a context option to disable automatic application of the
filter, and register the filter with the streams layer so that it can
be applied manually. (And of course, I'd second Stas' comment wrt
chunk-header length)
The issue wrt 1.0 versus 1.1 is fairly moot. If the caller doesn't
override the version then the server won't send chunked encoding.
They are capable of doing so however, and they may already be doing it
(e.g. for a streaming client). So the concern about potential BC
breakage for apps still exists, it's just less severe (since I doubt
many apps *are* explicitly setting the version to 1.1)
-Sara
Index: ext/standard/filters.c
===
RCS file: /repository/php-src/ext/standard/filters.c,v
retrieving revision 1.44.2.6.2.4.2.3
diff -u -p -d -r1.44.2.6.2.4.2.3 filters.c
--- ext/standard/filters.c 31 Dec 2008 11:15:45 - 1.44.2.6.2.4.2.3
+++ ext/standard/filters.c 15 Apr 2009 11:35:07 -
@@ -1897,6 +1897,220 @@ php_stream_filter_factory consumed_filte
/* }}} */
+/* {{{ chunked filter implementation */
+typedef enum _php_chunked_filter_state {
+ CHUNK_SIZE_START,
+ CHUNK_SIZE,
+ CHUNK_SIZE_EXT_START,
+ CHUNK_SIZE_EXT,
+ CHUNK_SIZE_CR,
+ CHUNK_SIZE_LF,
+ CHUNK_BODY,
+ CHUNK_BODY_CR,
+ CHUNK_BODY_LF,
+ CHUNK_TRAILER,
+ CHUNK_ERROR
+} php_chunked_filter_state;
+
+typedef struct _php_chunked_filter_data {
+ php_chunked_filter_state state;
+ int chunk_size;
+ int persistent;
+} php_chunked_filter_data;
+
+static int php_dechunk(char *buf, int len, php_chunked_filter_data *data)
+{
+ char *p = buf;
+ char *end = p + len;
+ char *out = buf;
+ int out_len = 0;
+
+ while (p end) {
+ switch (data-state) {
+ case CHUNK_SIZE_START:
+ data-chunk_size = 0;
+ case CHUNK_SIZE:
+ while (p end) {
+ if (*p = '0' *p = '9') {
+ data-chunk_size =
(data-chunk_size * 16) + (*p - '0');
+ } else if (*p = 'A' *p = 'F') {
+ data-chunk_size =
(data-chunk_size * 16) + (*p - 'A' + 10);
+ } else if (*p = 'a' *p = 'f') {
+ data-chunk_size =
(data-chunk_size * 16) + (*p - 'a' + 10);
+ } else if (data-state ==
CHUNK_SIZE_START) {
+ data-state = CHUNK_ERROR;
+ break;
+ } else {
+ data-state =
CHUNK_SIZE_EXT_START;
+ break;
+ }
+ data-state = CHUNK_SIZE;
+ p++;
+ }
+ if (data-state == CHUNK_ERROR) {
+ continue;
+ } else if (p == end) {
+ return out_len;
+ }
+ case CHUNK_SIZE_EXT_START:
+ if (*p == ';'|| *p == '\r' || *p == '\n') {
+ data-state = CHUNK_SIZE_EXT;
+
Re: [PHP-DEV] Support for Transfer-Encoding: chunked in http stream wrapper
I sent a new version of patch with these features a minute ago :) Thanks. Dmitry. Arnaud Le Blanc wrote: Hi, On Tue, 2009-04-14 at 18:59 +0400, Dmitry Stogov wrote: Some PHP applications which check for Transfer-Encoding HTTP header and perform manual decoding might be broken. What about not adding the Transfer-Encoding header to wrapper_data ? (not making it visible so that scripts would not break). Also, re-enabling the filters list in stream_get_meta_data() so that one can know that the dechunk filter has been added. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_3) / zend_extensions.c zend_extensions.h
Christopher Jones wrote: Stanislav Malyshev wrote: Hi! I'm looking forward to reading at least a mail note describing what extension maintainers can/can't do with this. Basically you can make your Zend extension load with any API number and any build ID (or let it decide with which ID to load and with which one not to). Prior to 5.3 and build IDs, you could do that for API number using api check callback, but not for other things like thread safety, etc. This is a dangerous functionality - as extension using this API should either not use most of the engine API or take care to use correct structures, etc. for every version. But for some applications it may be required. Is this Zend extensions only? Yes. Is it safe to set in extensions that should work with pre 5.3 PHP's? Yes, in php5.2 and below build_id just won't be checked ans this callback won't be called. Did I lose track of the other API versioning change - the one that was about to change the structure size. Or is this it but just applied to Zend extensions? It's applied only to zend_extensions and doesn't change the structure size as it has some reserved space. Thanks. Dmitry. Chris -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] double to long conversion change
Hi Matt,
I don't really see why we should preserve the least significant bits
and I don't think we should support bitwise operations with doubles.
Stas, could you please look into this too.
Thanks. Dmitry.
Matt Wilmas wrote:
Hi Dmitry,
- Original Message -
From: Dmitry Stogov
Sent: Thursday, April 02, 2009
Hi Matt,
I tried to look into this issue once again, but I completely
misunderstand why do we need all this magic. Why do we need conversion
of positive double into negative long?
I don't really have any more information than what has been given in my
various earlier messages that I've referenced. :-) But it's no problem!
It's probably too much to keep track of, or try to find which message I
said something in, I know (I have to do that myself to refresh memory
about some parts). So feel free to ask for an explanation about
anything. :-)
OK, regarding conversion of postive double into negative long (or it
could be positive if it rolls over again above ULONG_MAX, etc...): 1)
for me, the original issue I noticed, is preserving the least
significant bits when using bitwise AND on a large number (old ref
again: [1]). Although now I know the 5.2 behavior I was getting can't
be relied on (= ULONG_MAX it's probably OK however), that's what I'm
trying to do -- make conversions consistent and reliable. And 2)
unsigned specifiers in sprintf() (%u, %x, etc.) rely on this conversion
(though it currently *won't work* in 5.3 on 64-bit non-Windows). See
references in Bugs #30695 and #42868.
[1] http://marc.info/?l=php-internalsm=120799720922202w=2
The magic (different methods...?) is needed depending on what type of
conversion works on a platform. BTW, I wasn't satisfied with what I
ended up with for my patch (unsure about how some things would behave,
some guessing), so a few days ago I started to try coming up with
something more complete and precise depending on what works on a
platform. Not done yet, and will need to add some configure checks,
etc. (new for me).
I would stay with single DVAL_TO_LVAL() definition and use it in
places instead of (long)Z_DVAL().
That (single DVAL_TO_LVAL()) is basically what 5.2 has now until you
added more definitions (from Zoe) ;-) (which behave differently [2]) for
5.3 in Nov '07 for Bug #42868.
[2] http://marc.info/?l=php-internalsm=123496364812725w=2
#define DVAL_TO_LVAL(d, l) \
if ((d) LONG_MAX) { \
(l) = LONG_MAX; \
} else if ((d) LONG_MIN) { \
(l) = LONG_MIN; \
} else {\
(l) = (long) (d); \
}
That's close to 5.3's new version (depending which is used for a
platform), and *precisely* what was committed to zend_operators.c in Sep
'04 (v1.195 Resolve undefined behavior (joe at redhat) [3]). After
Bug #30695, it was reverted in Nov: v1.203 Revert Joe's work around a
bug in GCC patch as it breaks too many things. [4]
[3]
http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_operators.c?r1=1.194r2=1.195view=patch
[4]
http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_operators.c?r1=1.202r2=1.203view=patch
- Matt
Or may be we need a second macro for conversion into unsigned long
where it needed?
#define DVAL_TO_ULONG(d, l) \
if ((d) ULONG_MAX) { \
(l) = ULONG_MAX; \
} else if ((d) 0) { \
(l) = 0; \
} else {\
(l) = (unsigned long) (d); \
}
It also possible to add notices in case of overflow detection.
Thanks. Dmitry.
Matt Wilmas wrote:
Hi all,
Since noticing and reporting last year [1] different behavior when
casting out-of-range doubles to int after the DVAL_TO_LVAL() macro
was updated, I've wondered how to get the behavior I observed, and
thought could be relied on (that was wrong to think, since it was un-
or implementation-defined), back. And how to do so (what should be
expected?), while keeping in mind the reason for the change:
consistent behavior for tests. [2] Except that the current code does
not give consistent results, depending on which DVAL_TO_LVAL
definition is used on a platform. [3]
[1] http://marc.info/?l=php-internalsm=120799720922202w=2
[2] http://marc.info/?l=php-internalsm=123495655802226w=2
[3] http://marc.info/?l=php-internalsm=123496364812725w=2
So after I finally started to test my ideas for consistent/reliable
overflow across platforms a few days ago, I noticed that my
workaround technique quit working (0 instead of overflow) with
doubles over 2^63, without resorting to fmod(). That's on Windows,
but I suspect the same may happen on other systems that are limited
to 64-bit integer processing internally or something (32-bit
platforms?). On 64-bit Linux anyway, it looks like doubles 2^63 do
rollover as expected (128-bit internal processing?):
http://marc.info/?l=php-internalsm=123376495021789w=2
I wasn't sure how to rethink things after that... But of course with
doubles, precision has been lost long before 2^63 anyway, as far as
increments of 1 (it's 1024 at 2^63).
What I wound up with for now, is using 5.2's method on 64-bit
platforms
[PHP-DEV] Re: [PATCH] double to long conversion change
Hi Matt,
I tried to look into this issue once again, but I completely
misunderstand why do we need all this magic. Why do we need conversion
of positive double into negative long?
I would stay with single DVAL_TO_LVAL() definition and use it in places
instead of (long)Z_DVAL().
#define DVAL_TO_LVAL(d, l) \
if ((d) LONG_MAX) { \
(l) = LONG_MAX; \
} else if ((d) LONG_MIN) { \
(l) = LONG_MIN; \
} else {\
(l) = (long) (d); \
}
Or may be we need a second macro for conversion into unsigned long where
it needed?
#define DVAL_TO_ULONG(d, l) \
if ((d) ULONG_MAX) { \
(l) = ULONG_MAX; \
} else if ((d) 0) { \
(l) = 0; \
} else {\
(l) = (unsigned long) (d); \
}
It also possible to add notices in case of overflow detection.
Thanks. Dmitry.
Matt Wilmas wrote:
Hi all,
Since noticing and reporting last year [1] different behavior when
casting out-of-range doubles to int after the DVAL_TO_LVAL() macro was
updated, I've wondered how to get the behavior I observed, and thought
could be relied on (that was wrong to think, since it was un- or
implementation-defined), back. And how to do so (what should be
expected?), while keeping in mind the reason for the change: consistent
behavior for tests. [2] Except that the current code does not give
consistent results, depending on which DVAL_TO_LVAL definition is used
on a platform. [3]
[1] http://marc.info/?l=php-internalsm=120799720922202w=2
[2] http://marc.info/?l=php-internalsm=123495655802226w=2
[3] http://marc.info/?l=php-internalsm=123496364812725w=2
So after I finally started to test my ideas for consistent/reliable
overflow across platforms a few days ago, I noticed that my workaround
technique quit working (0 instead of overflow) with doubles over 2^63,
without resorting to fmod(). That's on Windows, but I suspect the same
may happen on other systems that are limited to 64-bit integer
processing internally or something (32-bit platforms?). On 64-bit Linux
anyway, it looks like doubles 2^63 do rollover as expected (128-bit
internal processing?):
http://marc.info/?l=php-internalsm=123376495021789w=2
I wasn't sure how to rethink things after that... But of course with
doubles, precision has been lost long before 2^63 anyway, as far as
increments of 1 (it's 1024 at 2^63).
What I wound up with for now, is using 5.2's method on 64-bit platforms,
and on 32-bit, overflow behavior should be reliable up to 2^63 on
platforms that have zend_long64 type available (long long, __int64),
which I'm guessing is most (?), because of the unsigned long
involvement. Finally a fallback workaround for 32-bit platforms without
a 64-bit type.
I updated a few other places in the code where only a (long) cast was
used. And sort of unrelated, but I added an 'L' conversion specifier for
zend_parse_parameters() in case it would be useful for PHP functions
that want to limit values to LONG_MAX/LONG_MIN, without overflow, which
I thought the DVAL_TO_LVAL change was *trying* to do.
http://realplain.com/php/dval_to_lval.diff
http://realplain.com/php/dval_to_lval_5_3.diff
And here is an initial version of zend_dval_to_lval() (before 2^63 issue
and thinking of zend_long64 + unsigned long), where some configure
checks would set ZEND_DVAL_TO_LVAL_USE_* as needed.
http://realplain.com/php/dval_to_lval.txt
Any general feedback, comments, questions, suggestions? Hoping these
conversion issues could be sorted out for good in a nice, logical way.
:-) Unfortunately on Windows, I'm just guessing, rather than testing,
conversion results in different environments...
Thanks,
Matt
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Compile-time hash value calculation (was: Patch for the php bug 47815)
Hi Basant,
I've updated you patch for 5.3 (attached, don't forget to regenerate
zend_vm_execute.h). 5.2 is closed for such changes anyway and I don't
think the patch should be applied into 5.3 too, because it is in RC stage.
I got ~10% speed-up on very synthetic benchmarks (b.php attached).
The speed difference on real-life applications without opcode caches is
invisible. I wasn't able to test it with caches.
In general the idea is very interesting, but as I mentioned before I
don't like extension of zend_opcode and tricky usage of object handlers.
It should be done in some more general way.
Anyway, please check if my changes didn't make your patch slower.
May be someone would like to play with patch and make it better...
Thanks. Dmitry.
Basant Kukreja wrote:
Attached is the patch for bug 47815
http://bugs.php.net/bug.php?id=47815
I applied this patch on php-5.2.9 and it applied cleanly.
Thanks for looking into the patch.
Regards,
Basant.
Index: Zend/zend_compile.h
===
RCS file: /repository/ZendEngine2/zend_compile.h,v
retrieving revision 1.316.2.8.2.12.2.39
diff -u -p -d -r1.316.2.8.2.12.2.39 zend_compile.h
--- Zend/zend_compile.h 26 Jan 2009 22:54:20 - 1.316.2.8.2.12.2.39
+++ Zend/zend_compile.h 31 Mar 2009 18:12:53 -
@@ -84,6 +84,7 @@ struct _zend_op {
znode op1;
znode op2;
ulong extended_value;
+ ulong hval;
uint lineno;
zend_uchar opcode;
};
Index: Zend/zend_execute.c
===
RCS file: /repository/ZendEngine2/zend_execute.c,v
retrieving revision 1.716.2.12.2.24.2.41
diff -u -p -d -r1.716.2.12.2.24.2.41 zend_execute.c
--- Zend/zend_execute.c 18 Mar 2009 14:15:28 - 1.716.2.12.2.24.2.41
+++ Zend/zend_execute.c 31 Mar 2009 18:12:53 -
@@ -784,7 +784,7 @@ static inline HashTable *zend_get_target
return NULL;
}
-static inline zval **zend_fetch_dimension_address_inner(HashTable *ht, const
zval *dim, int type TSRMLS_DC)
+static inline zval **zend_fetch_dimension_address_inner(HashTable *ht, const
zval *dim, int type, zend_ulong hval TSRMLS_DC)
{
zval **retval;
char *offset_key;
@@ -795,6 +795,7 @@ static inline zval **zend_fetch_dimensio
case IS_NULL:
offset_key = ;
offset_key_length = 0;
+ hval = zend_inline_hash_func(, 1);
goto fetch_string_dim;
case IS_STRING:
@@ -803,7 +804,7 @@ static inline zval **zend_fetch_dimensio
offset_key_length = dim-value.str.len;
fetch_string_dim:
- if (zend_symtable_find(ht, offset_key,
offset_key_length+1, (void **) retval) == FAILURE) {
+ if (zend_symtable_quick_find(ht, offset_key,
offset_key_length+1, hval, (void **) retval) == FAILURE) {
switch (type) {
case BP_VAR_R:
zend_error(E_NOTICE, Undefined
index: %s, offset_key);
@@ -819,7 +820,7 @@ fetch_string_dim:
zval *new_zval =
EG(uninitialized_zval);
Z_ADDREF_P(new_zval);
-
zend_symtable_update(ht, offset_key, offset_key_length+1, new_zval,
sizeof(zval *), (void **) retval);
+
zend_symtable_quick_update(ht, offset_key, offset_key_length+1, hval,
new_zval, sizeof(zval *), (void **) retval);
}
break;
}
@@ -867,7 +868,7 @@ num_index:
return retval;
}
-static void zend_fetch_dimension_address(temp_variable *result, zval
**container_ptr, zval *dim, int dim_is_tmp_var, int type TSRMLS_DC)
+static void zend_fetch_dimension_address(temp_variable *result, zval
**container_ptr, zval *dim, int dim_is_tmp_var, int type, zend_ulong hval
TSRMLS_DC)
{
zval *container = *container_ptr;
zval **retval;
@@ -890,7 +891,7 @@ fetch_from_array:
Z_DELREF_P(new_zval);
}
} else {
- retval =
zend_fetch_dimension_address_inner(Z_ARRVAL_P(container), dim, type TSRMLS_CC);
+ retval =
zend_fetch_dimension_address_inner(Z_ARRVAL_P(container), dim, type, hval
TSRMLS_CC);
}
result-var.ptr_ptr = retval;
PZVAL_LOCK(*retval);
@@ -1022,7 +1023,7 @@ convert_to_array:
}
}
-static void zend_fetch_dimension_address_read(temp_variable *result, zval
**container_ptr,
[PHP-DEV] Re: [ZEND-ENGINE-CVS] Re: [PHP-DEV] Re: cvs: ZendEngine2 / zend_builtin_functions.c /tests 009.phpt php-src/ext/reflection/tests 027.phpt php-src/ext/standard/tests/class_object get_class_
Lukas Kahwe Smith wrote:
On 16.03.2009, at 16:49, Pierre Joye wrote:
2009/3/16 Johannes Schlüter johan...@php.net:
Hi,
On Mon, 2009-03-16 at 09:52 +, Dmitry Stogov wrote:
Log:
Fixed bug #47664 (get_class returns NULL instead of FALSE)
[...]
@@ -716,7 +716,7 @@
int dup;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, |o,
obj) == FAILURE) {
- return;
+ RETURN_FALSE;
}
Usually we return NULL in case parameter parsing fails, this is
documented like this:
Hi Johannes,
Please read the bug report. In this case I think that we should not
break it only to follow this (arguable :) rule. See the documentation
of get_class as well: http://de.php.net/get_class
I did not follow this closely. So where do we stand here? Should we do a
quick vote? Or is this decided already? In this case did it get
documented (in case we decided to change the behavior).
The fix makes get_class() to confirm to documentation at
http://ru.php.net/manual/en/function.get-class.php (Returns FALSE if
object is not an object) and behave in the same way as 5.2.
Thanks. Dmitry.
If someone can summarize the current state of things I would appreciate it.
regards,
Lukas Kahwe Smith
m...@pooteeweet.org
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
Hi Matt, I ran you version but it looked little bit slower (I checked it with callgrind). So I kept the current CVS version. Anyway, it is not a big problem to change it after RC. Thanks. Dmitry. Matt Wilmas wrote: Hi Dmitry, - Original Message - From: Dmitry Stogov Sent: Thursday, March 19, 2009 Hi Matt, Matt Wilmas wrote: Hi again Dmitry, Just wanted to say that I think I can make your code a bit smaller after looking at it closer. It would be great, but keep in mind that performance is more important. I know. :-) Don't quite have enough time to modify and verify it now, but I'll be back later (around the usual time) to let you know either way. :-) I'm waiting for your changes. No patch, but here's the straight code: http://realplain.com/php/handle_numeric.txt With your benchmark script, if I changed one of the keys, it would sometimes radically affect the time of another key value, hmm. But in my own tests, this was always faster. Some notes: I've never understood why the terminating null check is there... I don't see why it would be missing if there aren't other problems in code. A place for UNEXPECTED()? I removed the special handling of 0 and it's faster. (Oh no, I see you just commited your code right now, LOL. Sorry I didn't send this sooner. :-/) For the leading 0 check, I changed it back to length 2 again instead of the end - tmp != 1 calculation. I used ++tmp instead of 2 *tmp++ because I assume having the increment in just one place creates less instructions. ;-) (For size, not speed.) Finally, in the Unicode version (copied, untested), I'm using the 0x values that were there originally instead of '0' char values... Thanks. Dmitry. Again, sorry I was about 20 minutes late with this! :-( - Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
Hi Matt, Matt Wilmas wrote: Hi again Dmitry, Just wanted to say that I think I can make your code a bit smaller after looking at it closer. It would be great, but keep in mind that performance is more important. Don't quite have enough time to modify and verify it now, but I'll be back later (around the usual time) to let you know either way. :-) I'm waiting for your changes. Thanks. Dmitry. - Matt - Original Message - From: Dmitry Stogov Sent: Wednesday, March 18, 2009 Hi Matt, I suppose I fixed the patch. Could you please check which patch is better yours or the attached one? According to attached benchmark my one is faster for most usual cases, but may be I forget something again. $a[abcdefghij] 0.130 0.130 $a[1234567890] 0.187 0.104 $a[2147483648] 0.168 0.142 $a[0]0.116 0.082 $a[214748364x] 0.136 0.141 $a[0] 0.080 0.080 Also, do you have other patches which I could look into before RC? Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
Hi Matt, I have no objections against proposed behaviour, but can't we use just a check for (errno == ERANGE) after strtol()? Thanks. Dmitry. Matt Wilmas wrote: Hi all, Assuming there are no objections, I'll commit this fix in a few hours... Besides the bug report(s), I had also found awhile ago that currently an array key can be LONG_MAX or LONG_MIN as a string and/or integer because of a check in ZEND_HANDLE_NUMERIC() (I assume to avoid a slow errno check for ERANGE originally). I changed it to use the *same method* that's used in the scanner, is_numeric_string(), etc., and those 2 values are now treated as an integer. It's just a few lines changed in zend_hash.h, although I had to move some of the definitions from zend_operators.h to zend.h (is that OK?). Patches (didn't check HEAD's yet): http://realplain.com/php/array_key_limit.diff http://realplain.com/php/array_key_limit_5_3.diff - Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
Hi Matt,
Matt Wilmas wrote:
Hi Dmitry,
- Original Message -
From: Dmitry Stogov
Sent: Wednesday, March 18, 2009
BTW may be we should eliminate strtol() at all.
There's no need to scan the string twice.
Your change to remove strtol() [1] is not checking for overflow
correctly (for example, zend_u_strtol()'s checks are more complicated).
This breaks handling of keys above ULONG_MAX (it's correct again after
ULONG_MAX+LONG_MAX, until ULONG_MAX * 2, etc.). See:
var_dump(array('50' = 1));
array(1) {
[705032704]=
int(1)
}
Thank you for catching this.
I'll think a bit and then revert patch if don't find better solution.
And of course the new code is a bit slower for keys that aren't fully
numeric, e.g. 123a
Agree, but it's not a usual case, 1234 would occur much faster.
Thanks. Dmitry.
[1] http://news.php.net/php.zend-engine.cvs/7465
Dmitry.
- Matt
Matt Wilmas wrote:
Hi all,
Assuming there are no objections, I'll commit this fix in a few hours...
Besides the bug report(s), I had also found awhile ago that currently
an array key can be LONG_MAX or LONG_MIN as a string and/or integer
because of a check in ZEND_HANDLE_NUMERIC() (I assume to avoid a slow
errno check for ERANGE originally). I changed it to use the *same
method* that's used in the scanner, is_numeric_string(), etc., and
those 2 values are now treated as an integer.
It's just a few lines changed in zend_hash.h, although I had to move
some of the definitions from zend_operators.h to zend.h (is that OK?).
Patches (didn't check HEAD's yet):
http://realplain.com/php/array_key_limit.diff
http://realplain.com/php/array_key_limit_5_3.diff
- Matt
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
Hi Matt,
I suppose I fixed the patch.
Could you please check which patch is better yours or the attached one?
According to attached benchmark my one is faster for most usual cases,
but may be I forget something again.
$a[abcdefghij] 0.130 0.130
$a[1234567890] 0.187 0.104
$a[2147483648] 0.168 0.142
$a[0]0.116 0.082
$a[214748364x] 0.136 0.141
$a[0] 0.0800.080
Also, do you have other patches which I could look into before RC?
Thanks. Dmitry.
Matt Wilmas wrote:
Hi Dmitry,
- Original Message -
From: Dmitry Stogov
Sent: Wednesday, March 18, 2009
BTW may be we should eliminate strtol() at all.
There's no need to scan the string twice.
Your change to remove strtol() [1] is not checking for overflow
correctly (for example, zend_u_strtol()'s checks are more complicated).
This breaks handling of keys above ULONG_MAX (it's correct again after
ULONG_MAX+LONG_MAX, until ULONG_MAX * 2, etc.). See:
var_dump(array('50' = 1));
array(1) {
[705032704]=
int(1)
}
And of course the new code is a bit slower for keys that aren't fully
numeric, e.g. 123a
[1] http://news.php.net/php.zend-engine.cvs/7465
Dmitry.
- Matt
Matt Wilmas wrote:
Hi all,
Assuming there are no objections, I'll commit this fix in a few hours...
Besides the bug report(s), I had also found awhile ago that currently
an array key can be LONG_MAX or LONG_MIN as a string and/or integer
because of a check in ZEND_HANDLE_NUMERIC() (I assume to avoid a slow
errno check for ERANGE originally). I changed it to use the *same
method* that's used in the scanner, is_numeric_string(), etc., and
those 2 values are now treated as an integer.
It's just a few lines changed in zend_hash.h, although I had to move
some of the definitions from zend_operators.h to zend.h (is that OK?).
Patches (didn't check HEAD's yet):
http://realplain.com/php/array_key_limit.diff
http://realplain.com/php/array_key_limit_5_3.diff
- Matt
Index: Zend/zend_hash.h
===
RCS file: /repository/ZendEngine2/zend_hash.h,v
retrieving revision 1.78.2.2.2.2.2.10
diff -u -p -d -r1.78.2.2.2.2.2.10 zend_hash.h
--- Zend/zend_hash.h18 Mar 2009 09:48:55 - 1.78.2.2.2.2.2.10
+++ Zend/zend_hash.h18 Mar 2009 17:05:53 -
@@ -306,18 +306,42 @@ END_EXTERN_C()
#define ZEND_HANDLE_NUMERIC(key, length, func) do {
\
register const char *tmp = key;
\
- const char *end = key + length - 1;
\
- long idx;
\
\
if (*tmp == '-') {
\
tmp++;
\
}
\
- if ((*tmp = '1' *tmp = '9' (end - tmp) MAX_LENGTH_OF_LONG) ||
\
- (*tmp == '0' (end - tmp) == 1)) {
\
- /* possibly a numeric index without leading zeroes */
\
- idx = (*tmp++ - '0');
\
- while (1) {
\
- if (tmp == end *tmp == '\0') { /* a numeric index */
\
+ if (*tmp = '0' *tmp = '9') { /* possibly a numeric index */
\
+ const char *end = key + length - 1;
\
+ long idx;
\
+
\
+ if (*end != '\0') { /* not a null terminated string */
\
+ break;
\
+ } else if (*tmp == '0
[PHP-DEV] Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
Matt Wilmas wrote:
Hi Dmitry,
- Original Message -
From: Dmitry Stogov
Sent: Wednesday, March 18, 2009
Hi Matt,
I suppose I fixed the patch.
Could you please check which patch is better yours or the attached one?
According to attached benchmark my one is faster for most usual cases,
but may be I forget something again.
$a[abcdefghij] 0.130 0.130
$a[1234567890] 0.187 0.104
$a[2147483648] 0.168 0.142
$a[0]0.116 0.082
$a[214748364x] 0.136 0.141
$a[0] 0.080 0.080
I gotta be away from the computer for a while in a second, so didn't
actually test your patch myself, but just wanted to say quick that it
looks good when I skimmed it. And those benchmarks look cool. :-)
ok. I'll wait till tomorrow.
Also, do you have other patches which I could look into before RC?
I think just the 2 I sent to the list a few days ago, and hoped to get
some feedback from others (haven't). If you didn't see them:
http://marc.info/?l=php-internalsm=123704111325725w=2
This is about double to long conversion, what should happen,
consistency, etc. that I've been wondering about since last year. All
info should be in that message (and linked ones).
http://marc.info/?l=php-internalsm=123722650225792w=2
Suggestion to have bitwise and modulus operators operate in 64-bit
mode if necessary on 32-bit platforms. Just a bit ago it came to mind
that I think I can improve this patch -- the behavior would be exactly
the same, just better, more optimized code. (I'll do that as soon as I
can, but it wouldn't affect you looking into it. I plan to eliminate
the big macro I added, FYI.)
I'll try to look into them too.
Thanks. Dmitry.
I'd imagine they could be discussed, etc. past the RC...
Thanks. Dmitry.
- Matt
Matt Wilmas wrote:
Hi Dmitry,
- Original Message -
From: Dmitry Stogov
Sent: Wednesday, March 18, 2009
BTW may be we should eliminate strtol() at all.
There's no need to scan the string twice.
Your change to remove strtol() [1] is not checking for overflow
correctly (for example, zend_u_strtol()'s checks are more complicated).
This breaks handling of keys above ULONG_MAX (it's correct again after
ULONG_MAX+LONG_MAX, until ULONG_MAX * 2, etc.). See:
var_dump(array('50' = 1));
array(1) {
[705032704]=
int(1)
}
And of course the new code is a bit slower for keys that aren't fully
numeric, e.g. 123a
[1] http://news.php.net/php.zend-engine.cvs/7465
Dmitry.
- Matt
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
