[PHP-INTERNALS] Re: [APC-DEV] [RFC] APC/PHP Lazy Loading

I did not read through the entire thread. As things are close to RC
state in 5.3, I would prefer to not do any non bug fixes at this
stage. Then again if the benefits are huge and the risk is low it can
be considered of course ..

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

Yep, I should have been clearer here. ;-) I don't believe this patch is ready to be committed and I'm not advocating for inclusion in the php53 release. I have some more adjustments I'd like to make to both the APC code and this patch, both for optimization, cleanliness, and to ensure it's the best possible implementation for future enhancements (such as lazy loading methods). I'm more interested in getting constructive feedback and any benchmark results so I can make adjustments to the current code. However, I would like to come back with improved patches and get it included in the next major release as appropriate.

I'm all about fixing the remaining php53 todos/bugs and getting this release out! I hope posting this wasn't a distraction from that.

Thanks!

-shire

Hi,

Personally, I like the patch except for some small possible tweaks, and
I believe it can't make any harm with lazy loading disabled.

Could you provide some benchmark results?
APC patch to play with it and see advantages/disadvantages?

Thanks. Dmitry.

shire wrote:

I was hoping to solicit some from others on the list, but haven't seen anything yet. My best example of gains is Facebook, I believe these where around 20-30% decrease in CPU usage on a bare-bones page.

I did test Joomla and Zend Framework, the gains here aren't much if anything as they seem to be use autoload for most files. (although I would like to see what lazy method loading can do here).

I intend to benchmark wordpress as well. I'll post more benchmarks for the above and this once I make some more tweaks that also might give us better results. I anticipate that benchmarks are going to vary pretty drastically depending on code structure, size, autoloading, etc.
I've gone ahead and checked in the current code for APC, so you'll have the necessary changes if you checkout CVS HEAD.

-shire

I thought about different semantics for defined_function_hook and
declared_class_hook to just load all "lazy" function/classes into
regular ZE tables.

It's also possible to pass hash_values into lookup_function_hook and
lookup_class_hook to don't calculate them twice.

I'll return to you in a day or two (after playing with patch and
measuring speed difference) to be more concrete.

Thanks. Dmitry.

Hi Shire,

I run patched APC on a number of real-life applications and got more
than 30% speedup on XOOPS (99 req/sec instead of 60%) and 20% on
ZendFramework (41 req/sec instead of 32), however most applications
(drupal, qdig, typo3, wordpress) didn't show significant speed
difference. As was expected the patch doesn't affects PHP without APC or
with APC and lazy loading disabled.

I also got APC warning with Zend Framewoek based blog application, but I
didn't try to look deeper.

[Wed Mar 11 17:53:02 2009] [apc-warning] [Wed Mar 11 17:53:02 2009]
[apc-warning] apc_lookup_class_hook: could not install blogrow in
/var/www/html/bench/fw/ZendFramework-1.5.0RC3/library/Zend/Loader.php on
line 86

I didn't look careful into APC code, just into PHP patch and I see the
following issues:

1) I would prefer to add additional hash_value argument into
lookup_function_hook() and lookup_class_hook to prevent multiple
calculation.

2) function_exists() should use lookup_function_hook().

3) interface_exists() and class_alias() should use lookup_class_hook().

4) ext/soap, ext/reflection, ext/wddx and ext/spl autoload support

Anyway, it's very good job and 20-30% speedup on some real-life
applications makes sense to include it into 5.3 (from my point of view).

Thanks. Dmitry.

shire wrote:

Makes sense to me as well, especially since I don't see any drawbacks
for non-accelerated scripts.

I also think some of those applications that didn't show any gains could
be trivially modified to make use of it. Moving from conditionally
loaded stuff to lazy-loaded is likely to speed things up. Depending of
course on how granular the conditional loading is.

-Rasmus

Can we get this patch to release quality by this weekend?
So that people can test it on Monday/Tuesday ahead of RC1?

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

I think it's possible. I could help with it.

Thanks. Dmitry.

I don't see this being a problem, I do have a few items I'd like to point out for feedback/suggestions:

1) Currently it doesn't support method level lazy loading, it's probably advisable to wait and include this later, but if everyone thinks it's significant enough we could probably add support for that. I'm not sure on all the details this involves, perhaps someone familiar with method calls could suggest difficulty and/or optimized ways to do the lookups.

2) Currently I've inserted these hook calls into everywhere we call/lookup classes. This has the downside that any extension wanting to mess with the function table, lookup entries, etc will need to be aware of the callback hooks. I think a better architecture is to create an API for function table and class table operations. Perhaps this could be done later if we want this likely more stable version in 5.3, and perhaps I'm overstating the significance of other functions doing these sort of things and not being aware of this new feature. (I believe dmitry mentions several extenions that need changes to support this). On the upside not creating an API means existing code will still work if they don't use lazy loading. ;-)

3) The largest portion of time currently is simply adding dummy entries to the lazy hash tables so we can detect name collisions and availability, my next goal is to improve the performance of this by either creating a faster copy of the entries or determine some better method of performing lookups/marking functions as available (something like lookups directly into the APC shared memory segment). Just putting this out there in case anyone has some interesting suggestions.


I think all the above 3 items don't necessarily need to be done to have this work in 5.3 (and #3 is pretty much APC/opcode cache centric) and might cause unecessary complication for an initial support of this, but what does everyone else think?



Regarding a couple of Dmitry's suggestions:
I'm pretty sure I already have support for this, it may have been lost while porting it over, I'll double check.


I'll follow up with Dmitry on getting these and any other items taken care of, thanks!

-shire

Hmm, thanks for your open assessment here. What you are saying does
make me wonder if we really should push this into 5.3. Even if any
changes we need to do can eventually be handled by APC, I do think
that there will be other extension authors that would also suffer from
us messing around with these internals all too much in every release.
So maybe we should wait until the next bigger step before introducing
this. Other people can apply the patch themselves if they really need
the performance in the mean time ..

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

Hi,

I've fixed all the issues I found in the original patch (attached).

However then I realized that speed-up was caused by bugs that leaded to
render pages in improper way. After fix the speed-up disappear. :(

So now I don't see any reason to include it into 5.3.

Thanks. Dmitry.

Lukas Kahwe Smith wrote:

hi,

Then I would rather have a beta2 and not a RC. This change while being
great may require more tweaks, and as Marcus suggested, we could push
APC in too while being at it. I'm all for it (both :).

Cheers,

Hi

2009/3/13 Pierre Joye <pierre.php@gmail.com>:

Well, according to Dmitry (didn't test it myself) this doesn't bring
real improvements so I don't think it's needed.
This was asked on IRC multiple times and always said something along the
lines "talk to the current APC maintainers and if they think it's worth
propose to internals" as the current APC maintainers can judge stability
and risks better and have a better knowledge about missing 5.3 support.
But nobody seemed t be that interested.

By looking at the bug tracker I see 63 open APC bugs including missing
support for 5.3 features (http://pecl.php.net/bugs/bug.php?id=15901) and
some crashes from that I think APC benefits from an independent release
schedule. Additionally APC doesn't offer core functionality and will
need configuration to be useful (temp path, shm method, segment
sizes, ...) so I don't think the additional installation step is that
much of additional trouble.

johannes

I want to emphasize that this does provide real performance increases, it's just going to depend heavily on the code structure. Some projects may see gains, others may not. Per my original post I'm interested in getting more testing done on various projects to see how general the gains are and what I can do to improve them.

To also re-iterate my previous emails I do agree that this probably isn't the best time to include this in 5.3, but would of course work to help get it in if the majority wanted to see that happen. Considering the merge errors that Dmitry found I think it would be preferred to get this tested more before it's included in a major release, as I had originally intended. I appreciate the code changes Dmitry has contributed and his initial testing.

I'm trying to get the majority of the APC bugs resolved, but I think this will take some time. There's also some new and undocumented functionality added to APC as well as some major changes I plan to create in an APC-4.0 branch. So I don't think this would be the best time to integrate it into core, especially considering the late notice.


-shire

Hello Lukas,

Wednesday, March 11, 2009, 5:10:57 PM, you wrote:
Right, basically the patch adresses what we say at conferences. Autload can
do an amazing job but also harm you. The patch on the other hand somehow
combines the advantages of both worlds. You can be explicit because you
know it better and for that you do not get any punishment from Autoload
execution time. And then again, you don't get punished by including too
much. Becuase after all the compiler knows it even better.

And of course, given that some people gain over 20%, I don't see why we
even need to discuss putting this in.

Last but not least, Lukas, what happened, to putting APC into core?

marcus




Best regards,
Marcus

That was planned for PHP 6.0.

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

Hello Lukas,

Wednesday, March 11, 2009, 11:15:23 PM, you wrote:







Best regards,
Marcus

Hi!
Can we please stop trying new big features each time we approach RC? 5.3
is not last PHP version ever, and it's long overdue. Can't we just
release it without putting more and more potentially unstable changes
into it, and then discuss other stuff for 5.4, 6.0 and whatever comes in
orderly manner?