Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Runtime JIT Proposals »

Runtime JIT Proposals

php.internals

Sara Golemon

19 years ago
For reasons best left on IRC, it looks like I'll be working on runtime JIT. To that end, I've come up with a few proposals of varying complexity and feature-set completeness: Option 1: Dump support for compile-time JIT and replace it with a call at runtime using the same semantics. Advantages: No change in the API (well, no further change anyway, Unicode support pretty much guarantees that things will change regardless). Disadvantages: Could someone be relying on compile-time JIT for something already? Maybe activation triggers an action which has to take place prior to script execution? For what I've seen JIT isn't in heavy use, but my perceptions on the topic aren't definitive. Option 2: Leave compile-time JIT alone, and add a second callback for runtime JIT. Advantages: Doesn't break BC, and offers extensions the chance to know that the code contains autoglobal references without actually having to act on them unless they're needed. Disadvantages: Adds to complexity/confusion by having two separate callbacks for essentially the same action. Option 3: Extend JIT signature with a "stage" parameter to indicate if the JIT trigger is occuring during compile-time or run-time. The callback can decide when/if it performs processing using current return value disarming semantics. Option 4: Include fetchtype and subelement during runtime JIT callback allowing JIT callback to only do work necessary to prepare for the read/write call being performed. e.g. int php_example_jit_callback(int str_type, zstr str, int str_len, int stage, zval *container, int fetch_type, zval *element); Where str_type/str/str_len indicate the name of the variable being JITed, stage is one of COMPILETIME or RUNTIME, container is the autoglobal itself. Fetch_type is ZEND_FETCH_(DIM_|OBJ_)?_(R|W|RW), and element is the specific property/offset (only applicable for DIM/OBJ fetches, NULL for plain fetch. Advantages: Gives maximum flexibility to the implementation. In the case of http request encoding, it allows the decoder to differentiate between requests for a single element and fetches which want to retreive the entire array (e.g. foreach). Disadvantages: Adds a lot of complexity to the fetching of autoglobals and qand effectively doubles the amount of callback work being done for autoglobal objects. Will also confuse implementers on what the difference between this fetch callback is and the (read|write)_(dimension|property) callbacks used by objects. In response to the suggestion to just turn $_REQUEST (et.al.) into objects with overloaded array access, the big danger there is that the following behavior would change: $postdata = $_REQUEST; foreach($postdata as $idx => $val) { $postdata[$idx] = some_filter_func($val); } Were $_REQUEST turned into an object with overloaded array access, these changes to $postdata would modify the values in the original $_REQUEST (due to the reference-like behavior of PHP5+ objects). Personally, I like Option 4, but then I like complexity. I can certainly see going for any of the others, but I want to go with something that the rest of the group can see being appropriately useful. If I can get something approaching a semi-consensus on direction, I can have an implementation (or a couple, depending on feelings on the matter) in a few days. -Sara

Andi Gutmans

19 years ago
Hi Sara, Sorry but I wasn't on IRC so I don't quite understand what you're trying to accomplish ;) Can you please explain? Once I understand what you're trying to accomplish I'll be more than happy to provide feedback to the options list. Thanks, Andi

Rasmus Lerdorf

19 years ago
Andi Gutmans wrote:
> Hi Sara, > > Sorry but I wasn't on IRC so I don't quite understand what you're trying to accomplish ;) > Can you please explain? Once I understand what you're trying to accomplish I'll be more than happy to provide feedback to the > options list.
Andrei/Pierre have a document on this stuff somewhere. I wasn't on irc either. But I did chat with Andrei about this a couple of weeks ago and the basic problem we are trying to solve is how to properly encode user GPC data. The 2 big problems with the current compile-time JIT approach we use to populate GPC arrays is that bubbling an encoding error up to the user is a PITA, especially if we get an error on an entry the user didn't ask for since current JIT populates the entire array when any element in the array is accessed. And the second problem is that encoding is more expensive per element now, so populating the entire array just because the script tries to read one element could cause performance problems and could lead to the array poisoning I talked about in my reply to Sara's message. -Rasmus

Rasmus Lerdorf

19 years ago
Sara Golemon wrote:
> For reasons best left on IRC, it looks like I'll be working on runtime > JIT. To that end, I've come up with a few proposals of varying > complexity and feature-set completeness: > > Option 1: > Dump support for compile-time JIT and replace it with a call at runtime > using the same semantics. > > Advantages: No change in the API (well, no further change anyway, > Unicode support pretty much guarantees that things will change regardless). > > Disadvantages: Could someone be relying on compile-time JIT for > something already? Maybe activation triggers an action which has to > take place prior to script execution? For what I've seen JIT isn't in > heavy use, but my perceptions on the topic aren't definitive.
I have a feeling this won't break much, if anything, but I am not sure this is the best approach for Unicode encoding (see my response to Option 4).
> Option 2: > Leave compile-time JIT alone, and add a second callback for runtime JIT. > > Advantages: Doesn't break BC, and offers extensions the chance to know > that the code contains autoglobal references without actually having to > act on them unless they're needed. > > Disadvantages: Adds to complexity/confusion by having two separate > callbacks for essentially the same action.
What would compile-time JIT do here? Just create a bunch of binary elements that are then overwritten at runtime with the encoded elements on access? This doesn't seem like a good idea either as the compile-time version would almost always be completely redundant, wouldn't it?
> Option 3: > Extend JIT signature with a "stage" parameter to indicate if the JIT > trigger is occuring during compile-time or run-time. The callback can > decide when/if it performs processing using current return value > disarming semantics.
I think we'd confuse people with that. We should pick one and stick with it.
> Option 4: > Include fetchtype and subelement during runtime JIT callback allowing > JIT callback to only do work necessary to prepare for the read/write > call being performed.
I like this approach. Getting right down to the individual GPC entries avoids what could potentially be crippling overhead iterating through a lot of fields which may never be used. It also solves the issue of what to do in case of a conversion error. When you convert an entire array at once as current compile-time JIT does, what happens when a single entry has a conversion error? How do you propogate the error to the user? And what if the error is on an element the user doesn't care about? In fact, a bad guy could simply add random elements full of bogus data to trigger these errors. By taking this approach we avoid these poisonous entries and any encoding errors can be reported back to the user right when they happen. When you toss error handling into the mix I don't think this is the most complex solution as you indicated. I think this actually simplifies things a lot. -Rasmus

Pierre Joye

19 years ago
Hello Sara, On 1/15/07, Sara Golemon <pollita@php.net> wrote:
> For reasons best left on IRC, it looks like I'll be working on runtime > JIT. To that end, I've come up with a few proposals of varying > complexity and feature-set completeness: > > Option 1: > Dump support for compile-time JIT and replace it with a call at runtime > using the same semantics. > > Advantages: No change in the API (well, no further change anyway, > Unicode support pretty much guarantees that things will change regardless). > > Disadvantages: Could someone be relying on compile-time JIT for > something already? Maybe activation triggers an action which has to > take place prior to script execution? For what I've seen JIT isn't in > heavy use, but my perceptions on the topic aren't definitive.
As I told you, there was already a consensus on this solution, check my initial proposal (solution #2): http://news.php.net/php.internals/26965
> In response to the suggestion to just turn $_REQUEST (et.al.) into > objects with overloaded array access, the big danger there is that the > following behavior would change:
It will bring a BC break as well or is_array($arrayaccessobject) will have to return true and we have to be sure about its implementation (like properties access not always working well).
> Personally, I like Option 4, but then I like complexity. I can > certainly see going for any of the others, but I want to go with > something that the rest of the group can see being appropriately useful.
I like my initial proposal. All it needs is an extra function and to move the JIT management to runtime. The complexity is the same as what we have now. --Pierre

Andrei Zmievski

19 years ago
I like Option 4. -Andrei On Jan 14, 2007, at 8:24 PM, Sara Golemon wrote:

Sara Golemon

19 years ago
> Option 4: > Include fetchtype and subelement during runtime JIT callback allowing > JIT callback to only do work necessary to prepare for the read/write > call being performed. > > e.g. > > int php_example_jit_callback(int str_type, zstr str, int str_len, > int stage, zval *container, int fetch_type, zval *element); > > Where str_type/str/str_len indicate the name of the variable being > JITed, stage is one of COMPILETIME or RUNTIME, container is the > autoglobal itself. > Fetch_type is ZEND_FETCH_(DIM_|OBJ_)?_(R|W|RW), and element is the > specific property/offset (only applicable for DIM/OBJ fetches, NULL for > plain fetch. > > Advantages: Gives maximum flexibility to the implementation. In the > case of http request encoding, it allows the decoder to differentiate > between requests for a single element and fetches which want to retreive > the entire array (e.g. foreach). > > Disadvantages: Adds a lot of complexity to the fetching of autoglobals > and qand effectively doubles the amount of callback work being done for > autoglobal objects. Will also confuse implementers on what the > difference between this fetch callback is and the > (read|write)_(dimension|property) callbacks used by objects. > >
Okay, in attempting an implementation of this, I got reminded none-too-gently by the engine that it's not quite as simple as I'd remembered: <?php $g = $_GET; $f = $_POST['foo']; ?> compiled vars: !0 = $g, !1 = $f line # op fetch operands ---------------------------------------------- 2 0 FETCH_R global $0, '_GET' 1 ASSIGN $1, !0, $0 3 2 FETCH_R global $2, '_POST' 3 FETCH_DIM_R $3, $2, 'foo' 4 ASSIGN $4, !1, $3 Autoglobals aren't treated as CVs (as, for some reason, I was thinking they were) so at the time of initial fetch, it's difficult to know if the whole var is being fetched (as in the case of the line 2 assignment, or if it's being fetched so that a subelement can be fetched later (as in the case of the line 3 assignment). The solution I'm tempted to pursue for this is to back up yet another step and make autoglobals be CVs by extending the zend_compiled_variable struct to contain a flag indicating how the var should be fetched (the determination for which happens during fetch_simple_var during the compilation. This would then yield an opcode stack like the following: compiled vars: !0* = $_GET, !1 = $g, !2* = $_POST, !3 = $f line # op fetch operands ---------------------------------------------- 2 0 ASSIGN $0, !1, !0* 3 1 FETCH_DIM_R $1, !2*, 'foo' 2 ASSIGN $2, !3, $1 (The * notation indicating that cv->fetch_type == ZEND_FETCH_GLOBAL) Once that's applied (basicly as a stand-alone speed improvement, since globals are turned into CV fetches), the RT-JIT can be done using the plan I'd already formulated for Option 4. *THEN* we can apply runtime-JIT to http input encoding detection. Just keeping the conversation in the open and hoping anyone which critiques will voice them sooner rather than later. -Sara Two steps forward, one step back.

Sara Golemon

19 years ago
> The solution I'm tempted to pursue for this is to back up yet another > step and make autoglobals be CVs by extending the zend_compiled_variable > struct to contain a flag indicating how the var should be fetched (the > determination for which happens during fetch_simple_var during the > compilation. This would then yield an opcode stack like the following: >
Okay, here's a shockingly simple patch for allowing auto globals to be treated as CVs. The one question mark I've got in here is: Why the last check in fetch_simple_var_ex() for the ZEND_BEGIN_SILENCE opcode? This seems completely unnecessary from what I can tell and shouldn't bar a variable (global or not) from being treated as a CV... Am I missing something really obvious? -Sara

Dmitry Stogov

19 years ago
Hi Sara, It is interesting and very clear patch. Probably you idea can be extended to support regular globals too. I mean $GLOBALS["name"]. BTW I am not sure this patch will give significant speedup, because locals are used most often then globals, and your patch adds small overhead for them. Did you benchmarked your patch? According to ZEND_BEGIN_SILENCE, just try to run simple script <?php $x = @$y;?> with and without this check. In case of missing check, real $y fetch is performed in ASSIGN operator after ZEND_END_SILENCE, so we see error message. Thanks. Dmitry.

Sara Golemon

19 years ago
> It is interesting and very clear patch. > Probably you idea can be extended to support regular globals too. I mean > $GLOBALS["name"]. >
GLOBALS is itself in the auto global registry, so these would automatically get picked up too. Oh, no, you probably mean have that entire expression boil down to a CV, ah... that'll be slightly tricker (since it'll involve rewriting the opcode), but in principle it shouldn't be impossible. Even if it's ugly to do in the single-pass compiler, an opcode optimizer could certainly handle it in its second pass once the fetch_type is added to CV definitions...
> BTW I am not sure this patch will give significant speedup, because locals > are used most often then globals, and your patch adds small overhead for > them. > Did you benchmarked your patch? >
Well, I gave it the usual BogoMIPS treatment (Bogus Meaningless Indication of Processor Speed for anyone who doesn't recognize that), and I got the following encouraging numbers: <?php $_POST['foo'] = 'bar'; for($i = 0; $i<1000000; $i++) $b = $_POST['foo']; ?> [current -- without patch] $ time sapi/cli/php -f test.php real 0m1.057s user 0m0.660s sys 0m0.000s [with patch] $ time sapi/cli/php -f test.php real 0m0.490s user 0m0.467s sys 0m0.000s Even looking at just the user times, the gain is in the right direction and is a fair percentage...
> According to ZEND_BEGIN_SILENCE, just try to run simple script <?php $x = > @$y;?> with and without this check. > In case of missing check, real $y fetch is performed in ASSIGN operator > after ZEND_END_SILENCE, so we see error message. >
Ah, of course, that makes perfect sense... And I've got to say, that's a clever solution to that problem. It does unnecessarily rob DIM/OBJ fetches in a localized silence ($foo = @$bar[$baz];) of being CV when those technically could, but that's a fairly small price to pay for the elegance of the solution. So leaving the silence alone (since it's necessary), and considering the appearant gain for a minor code change, what are your thoughts on tossing this in as is and looking at extending it for $GLOBALS['foo'] -> (global fetch CV)$foo later on? -Sara

Dmitry Stogov

19 years ago
Hi Sara,
> -----Original Message----- > From: Sara Golemon [mailto:pollita@php.net] > Sent: Wednesday, January 17, 2007 11:36 PM > To: Dmitry Stogov > Cc: internals@lists.php.net; Andrei Zmievski; Andi Gutmans > Subject: Re: [PHP-DEV] Giving Globals the CV treatment [WAS: > Runtime JIT Proposals] > > > > It is interesting and very clear patch. > > Probably you idea can be extended to support regular globals too. I > > mean $GLOBALS["name"]. > > > GLOBALS is itself in the auto global registry, so these would > automatically get picked up too. Oh, no, you probably mean > have that > entire expression boil down to a CV, ah... that'll be > slightly tricker > (since it'll involve rewriting the opcode), but in principle it > shouldn't be impossible. Even if it's ugly to do in the single-pass > compiler, an opcode optimizer could certainly handle it in its second > pass once the fetch_type is added to CV definitions... > > > BTW I am not sure this patch will give significant speedup, because > > locals are used most often then globals, and your patch adds small > > overhead for them. Did you benchmarked your patch? > > > Well, I gave it the usual BogoMIPS treatment (Bogus Meaningless > Indication of Processor Speed for anyone who doesn't recognize that), > and I got the following encouraging numbers: > > <?php > $_POST['foo'] = 'bar'; > > for($i = 0; $i<1000000; $i++) > $b = $_POST['foo']; > ?> > > [current -- without patch] > $ time sapi/cli/php -f test.php > real 0m1.057s > user 0m0.660s > sys 0m0.000s > > > [with patch] > $ time sapi/cli/php -f test.php > real 0m0.490s > user 0m0.467s > sys 0m0.000s > > Even looking at just the user times, the gain is in the right > direction > and is a fair percentage...
So your patch makes sense. Could you also run Zend/bench.php to check that patch doesn't slowdown local fetches. I think the patch can be commited into HEAD (not into PHP_5_2), but I would prefer collect all performance patches and commit them into PHP_5_3 and HEAD together.
> > According to ZEND_BEGIN_SILENCE, just try to run simple > script <?php > > $x = @$y;?> with and without this check. In case of missing check, > > real $y fetch is performed in ASSIGN operator after > ZEND_END_SILENCE, > > so we see error message. > > > Ah, of course, that makes perfect sense... And I've got to > say, that's > a clever solution to that problem. It does unnecessarily rob DIM/OBJ > fetches in a localized silence ($foo = @$bar[$baz];) of being CV when > those technically could, but that's a fairly small price to > pay for the > elegance of the solution.
I would be glad to look into another solution, but I cannot even imagine it. PHP language wasn't designed to be fast :(
> So leaving the silence alone (since it's necessary), and > considering the > appearant gain for a minor code change, what are your thoughts on > tossing this in as is and looking at extending it for > $GLOBALS['foo'] -> > (global fetch CV)$foo later on?
Agree. See above. Dmitry.

Sara Golemon

19 years ago
> Could you also run Zend/bench.php to check that patch doesn't slowdown local > fetches. > I think the patch can be commited into HEAD (not into PHP_5_2), but I would > prefer collect all performance patches and commit them into PHP_5_3 and HEAD > together. >
without with simple 0.538 0.550 simplecall 2.046 1.932 simpleucall 2.956 2.885 simpleudcall 3.514 3.484 mandel 1.952 1.969 mandel2 3.398 3.332 ackermann(7) 3.358 3.364 ary(50000) 0.158 0.159 ary2(50000) 0.161 0.137 ary3(2000) 1.081 1.057 fibo(30) 9.587 9.618 hash1(50000) 0.422 0.416 hash2(500) 0.305 0.306 heapsort(20000) 0.779 0.804 matrix(20) 0.525 0.509 nestedloop(12) 0.931 0.927 sieve(30) 0.640 0.638 strcat(200000) 0.286 0.283 ------------------------------- Total 32.637 32.371 A net gain of about 1%. Of course, that number should be taken with a grain of salt in both directions as the test framework measures realtime, and not process time, but the numbers such as they are, *are* still pointing in the right direction...
> I would be glad to look into another solution, but I cannot even imagine it. > PHP language wasn't designed to be fast :( >
I'd say this can be resolved by an optimizer, but I'm going to try a few attempts and see if anything non-ugly can be come up with... $foo = @$_REQUEST['foo']; *is* a common enough usage that a solution would be worthwhile.
>> So leaving the silence alone (since it's necessary), and >> considering the >> appearant gain for a minor code change, what are your thoughts on >> tossing this in as is and looking at extending it for >> $GLOBALS['foo'] -> >> (global fetch CV)$foo later on? >
Same with this one, let an optimizer figure it out. Once the framework for specifying fetchtype is in place, the main engine compiler can take it's hands off the problem. -Sara

Ilia A.

19 years ago
The optimizer already does most of those things, so I think things like that are best left there rather then being done in the core. On 18-Jan-07, at 4:34 PM, Sara Golemon wrote:
>> Could you also run Zend/bench.php to check that patch doesn't >> slowdown local >> fetches. >> I think the patch can be commited into HEAD (not into PHP_5_2), >> but I would >> prefer collect all performance patches and commit them into >> PHP_5_3 and HEAD >> together. >> > without with > simple 0.538 0.550 > simplecall 2.046 1.932 > simpleucall 2.956 2.885 > simpleudcall 3.514 3.484 > mandel 1.952 1.969 > mandel2 3.398 3.332 > ackermann(7) 3.358 3.364 > ary(50000) 0.158 0.159 > ary2(50000) 0.161 0.137 > ary3(2000) 1.081 1.057 > fibo(30) 9.587 9.618 > hash1(50000) 0.422 0.416 > hash2(500) 0.305 0.306 > heapsort(20000) 0.779 0.804 > matrix(20) 0.525 0.509 > nestedloop(12) 0.931 0.927 > sieve(30) 0.640 0.638 > strcat(200000) 0.286 0.283 > ------------------------------- > Total 32.637 32.371 > > A net gain of about 1%. Of course, that number should be taken > with a grain of salt in both directions as the test framework > measures realtime, and not process time, but the numbers such as > they are, *are* still pointing in the right direction... > >> I would be glad to look into another solution, but I cannot even >> imagine it. >> PHP language wasn't designed to be fast :( > I'd say this can be resolved by an optimizer, but I'm going to try > a few attempts and see if anything non-ugly can be come up with... > $foo = @$_REQUEST['foo']; *is* a common enough usage that a > solution would be worthwhile. > >>> So leaving the silence alone (since it's necessary), and >>> considering the appearant gain for a minor code change, what are >>> your thoughts on tossing this in as is and looking at extending >>> it for $GLOBALS['foo'] -> (global fetch CV)$foo later on? > Same with this one, let an optimizer figure it out. Once the > framework for specifying fetchtype is in place, the main engine > compiler can take it's hands off the problem. > > -Sara > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Ilia Alshanetsky

Dmitry Stogov

19 years ago
1% is a measure mistake, so patch is OK. Dmitry.

Sara Golemon

19 years ago
One last thought on global CVs... How's this for fixing the loophole of: ZEND_BEGIN_SILENCE ZEND_FETCH_R $0 '_POST' (global) ZEND_FETCH_DIM_R $1 $0 'foo' ZEND_END_SILENCE I know it seems like a pretty minor gain, but (A) using @$_GET['foo'] isn't an uncommon practice, and (B) pushing this all into CV based FETCH_DIM ops would greatly simplify the runtime JIT stuff I'm working on. The short-version summary of this patch is that when the engine is going to do a FETCH_DIM based on prior nodes, it checks the top of the backpatch stack to see where the container element is coming from, if it's coming from a simple FETCH, it rewrites that to a FETCH_DIM, rather than adding a new element to the BP stack. -Sara P.S. - Bench numbers follow (0.1% difference): unpatched patched simple 0.472 0.473 simplecall 1.914 1.916 simpleucall 2.920 2.920 simpleudcall 3.507 3.507 mandel 2.056 2.055 mandel2 3.273 3.277 ackermann(7) 3.389 3.390 ary(50000) 0.157 0.157 ary2(50000) 0.138 0.138 ary3(2000) 1.057 1.056 fibo(30) 9.789 9.750 hash1(50000) 0.395 0.396 hash2(500) 0.300 0.300 heapsort(20000) 0.770 0.771 matrix(20) 0.508 0.508 nestedloop(12) 0.872 0.872 sieve(30) 0.640 0.640 strcat(200000) 0.289 0.289 -------------------------------- Total 32.446 32.416

Ilia A.

19 years ago
Given they way-below margin of error difference I have to wonder if perhaps the added "parse" times would more then offset any (if any) benefits gained. Given that most users still do not use opcode caches this is something to consider for performance tweaks where the difference is >1%. On 20-Jan-07, at 4:57 PM, Sara Golemon wrote:
> One last thought on global CVs... How's this for fixing the > loophole of: > > ZEND_BEGIN_SILENCE > ZEND_FETCH_R $0 '_POST' (global) > ZEND_FETCH_DIM_R $1 $0 'foo' > ZEND_END_SILENCE > > I know it seems like a pretty minor gain, but (A) using @$_GET > ['foo'] isn't an uncommon practice, and (B) pushing this all into > CV based FETCH_DIM ops would greatly simplify the runtime JIT stuff > I'm working on. > > The short-version summary of this patch is that when the engine is > going to do a FETCH_DIM based on prior nodes, it checks the top of > the backpatch stack to see where the container element is coming > from, if it's coming from a simple FETCH, it rewrites that to a > FETCH_DIM, rather than adding a new element to the BP stack. > > -Sara > > P.S. - Bench numbers follow (0.1% difference): > > unpatched patched > simple 0.472 0.473 > simplecall 1.914 1.916 > simpleucall 2.920 2.920 > simpleudcall 3.507 3.507 > mandel 2.056 2.055 > mandel2 3.273 3.277 > ackermann(7) 3.389 3.390 > ary(50000) 0.157 0.157 > ary2(50000) 0.138 0.138 > ary3(2000) 1.057 1.056 > fibo(30) 9.789 9.750 > hash1(50000) 0.395 0.396 > hash2(500) 0.300 0.300 > heapsort(20000) 0.770 0.771 > matrix(20) 0.508 0.508 > nestedloop(12) 0.872 0.872 > sieve(30) 0.640 0.640 > strcat(200000) 0.289 0.289 > -------------------------------- > Total 32.446 32.416 > > Index: Zend/zend_compile.c > =================================================================== > RCS file: /repository/ZendEngine2/zend_compile.c,v > retrieving revision 1.736 > diff -u -p -r1.736 zend_compile.c > --- Zend/zend_compile.c 20 Jan 2007 20:36:55 -0000 1.736 > +++ Zend/zend_compile.c 20 Jan 2007 21:55:50 -0000 > @@ -489,9 +489,33 @@ void fetch_array_begin(znode *result, zn > > void fetch_array_dim(znode *result, znode *parent, znode *dim > TSRMLS_DC) > { > - zend_op opline; > + zend_op opline, *parentop; > zend_llist *fetch_list_ptr; > > + zend_stack_top(&CG(bp_stack), (void **) &fetch_list_ptr); > + parentop = zend_llist_get_last(fetch_list_ptr); > + if (parentop && parent->op_type == IS_VAR && > + parentop->opcode == ZEND_FETCH_W && > + parentop->op1.op_type == IS_CONST && > + (Z_TYPE(parentop->op1.u.constant) == IS_STRING || Z_TYPE > (parentop->op1.u.constant) == IS_UNICODE) && > + !(Z_UNILEN(parentop->op1.u.constant) == (sizeof("this")-1) && > ZEND_U_EQUAL(Z_TYPE(parentop->op1.u.constant), Z_UNIVAL(parentop- > >op1.u.constant), Z_UNILEN(parentop->op1.u.constant), "this", sizeof > ("this")-1)) ) { > + /* Recompile CV and rewrite previous op to direct FETCH_DIM */ > + zval tmp = parentop->op1.u.constant; > + parentop->opcode = ZEND_FETCH_DIM_W; > + parentop->op1.op_type = IS_CV; > + parentop->op1.u.var = lookup_cv(CG(active_op_array), Z_TYPE > (tmp), Z_UNIVAL(tmp), Z_UNILEN(tmp) TSRMLS_CC); > + parentop->op1.u.EA.type = 0; > + parentop->op2 = *dim; > + parentop->extended_value = ZEND_FETCH_STANDARD; > + *result = parentop->result; > + > + /* Give temp var back if it was the most recently assigned only */ > + if (CG(active_op_array)->T == (parent->u.var - 1)) { > + CG(active_op_array)->T--; > + } > + return; > + } > + > init_op(&opline TSRMLS_CC); > opline.opcode = ZEND_FETCH_DIM_W; /* the backpatching routine > assumes W */ > opline.result.op_type = IS_VAR; > @@ -502,7 +526,6 @@ void fetch_array_dim(znode *result, znod > opline.extended_value = ZEND_FETCH_STANDARD; > *result = opline.result; > > - zend_stack_top(&CG(bp_stack), (void **) &fetch_list_ptr); > zend_llist_add_element(fetch_list_ptr, &opline); > } > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
Ilia Alshanetsky

Sara Golemon

19 years ago
> + /* Give temp var back if it was the most recently assigned only */ > + if (CG(active_op_array)->T == (parent->u.var - 1)) { > + CG(active_op_array)->T--; > + }
Ergh, ignore that section.... It doesn't belong there (obviously)... I pasted the wrong version of this patch :) -Sara

Dmitry Stogov

19 years ago
Hi Sara, At first I don't understand why you are trying to deallocate variable. + if (CG(active_op_array)->T == (parent->u.var - 1)) { + CG(active_op_array)->T--; + } Isn't the same variable reused as result of ZEND_FETCH_DIM? The rest of patch seems proper, but I am not sure about place. Why we optimize @$a['b'] but not @$a->b and @$a->b()? I would prefer to find more general solution. Dmitry.