Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » FW: Patch for macros for tracking refcount and is_ref »

FW: Patch for macros for tracking refcount and is_ref

php.internals

Andi Gutmans

18 years ago
When we all get a copy of the patch and get a chance to test it seriously we'll know more. I'm all in favor of having GC and very enthusiastic about this coming about but I doubt you can call it well enough tested. When we made memory manager changes in the past there were bugs which were only found after a public release. Even RCs and many people downloading it didn't help. In this case I still haven't had a chance to even play with the patch which I understand will be happening in a few days... If you have had a chance that's great but there are lots of others who want to too. The early patch I saw still needed work. Also, if GC slows certain situations down there may well be companies who would prefer to run without it. Facebook is probably a good example of a company where every % matters. The #if ZEND_GC is just an intermediate step so that we can commit the macro patch and get the GC patch for the list to review, play around with and test. It also gives us the flexibility to release a 5.2.x release if needed (urgent security fix). Once we get to the point where everyone feels comfortable with GC which shouldn't take too much time if it works as advertised we should discuss exactly how we want to handle it (enabled by default, configurable, how we handle compat with PECL). These things may be obvious to you but they aren't obvious to me right now. It may also very well be that after initial review we can make the default build GC, at least for the dev period so that we make sure it gets as much testing by ppl grabbing from CVS as possible. But let's first get the macro patch commited, get the GC patch against that and have a review period... Andi

Antony Dovgal

18 years ago
On 08.09.2007 19:34, Andi Gutmans wrote:
> When we all get a copy of the patch and get a chance to test it seriously we'll know more. > I'm all in favor of having GC and very enthusiastic about this coming about but I doubt you can > call it well enough tested. When we made memory manager changes in the past there were bugs > which were only found after a public release.
Agree, real life tests are proven to be much more effective than any synthetic one can imagine.
> Also, if GC slows certain situations down there may well be companies who would prefer to run without it. > Facebook is probably a good example of a company where every % matters.
If these companies really do worry about this %, why don't they participate in development and/or testing? I don't recall seeing any contributions or even feedback from Facebook or alike, do you? If they wait for a release to test and complain, then I couldn't care less - it's just a common development process, bugs come and bugs go, I see no reasons why we should make an exception for a company that doesn't give a damn about us.
-- Wbr, Antony Dovgal

Andi Gutmans

18 years ago
Uhm I didn't mean to make this about one company or another (it was just an example, and btw, I think they've contributed to APC :) and this discussion is going very much of track. I guess you either didn't understand what I was saying or don't realize that this is not necessarily going to be applicable to everyone (although it may depending on how it performs; and I hope it is applicable to everyone). I'm also just saying that it may need some time to mature like many patches which are so deep. Btw, I'm not talking about "synthetic" tests like bench.php or like the test script David wrote. I'm talking about taking real-world apps and testing them in a cluster under load. That can give some good insight... Anyway, I suggest to end this pointless discussion right now until we actually get a chance to play around with it. We're running on empty... Andi

Antony Dovgal

18 years ago
On 08.09.2007 21:31, Andi Gutmans wrote:
> Uhm I didn't mean to make this about one company or another (it was just an example, > and btw, I think they've contributed to APC :) and this discussion is going very much > of track. I guess you either didn't understand what I was saying or don't realize > that this is not necessarily going to be applicable to everyone (although it may > depending on how it performs; and I hope it is applicable to everyone).
Well, it was difficult to understand it wrong: you said that it's a major change and it should be tested well (I completely agree with that). But I don't see why we should especially care of a particular company which MIGHT be affected by this patch. Either they care about it and help us with to make it better, or we do & test it as good as _we_ can.
> I'm also just > saying that it may need some time to mature like many patches which are so deep.
Sure, that's obvious.
-- Wbr, Antony Dovgal

Rasmus Lerdorf

18 years ago
Antony Dovgal wrote:
> On 08.09.2007 19:34, Andi Gutmans wrote: >> When we all get a copy of the patch and get a chance to test it seriously we'll know more. >> I'm all in favor of having GC and very enthusiastic about this coming about but I doubt you can >> call it well enough tested. When we made memory manager changes in the past there were bugs >> which were only found after a public release. > > Agree, real life tests are proven to be much more effective than any synthetic one can imagine. > >> Also, if GC slows certain situations down there may well be companies who would prefer to run without it. >> Facebook is probably a good example of a company where every % matters. > > If these companies really do worry about this %, why don't they participate in development and/or testing? > I don't recall seeing any contributions or even feedback from Facebook or alike, do you?
Facebook is quite active in APC development. And yes, if it turns out to be 10% slower, a lot of sites are going to want to run without it. For scripts that aren't very complex and don't run for a long time, GC simply isn't a very interesting feature. -Rasmus

Antony Dovgal

18 years ago
On 08.09.2007 21:31, Rasmus Lerdorf wrote:
> Facebook is quite active in APC development.
I'm glad for APC, but this is a separate project (or a sub-project, whatever).
> And yes, if it turns out to be 10% slower, a lot of sites are going to want to run without it.
Right, but saying that we should be especially careful because this patch may affect company XXX makes no sense to me. Either this company is interested in this patch and helps us to make it better, or we follow the usual development process (i.e. try to make it as better as we can and release when it's ready, but we don't test it for years to stabilize it).
> For scripts that aren't very complex and don't run for a long time, GC > simply isn't a very interesting feature.
Well, for not very complex scripts most of the engine improvements are not very interesting.
-- Wbr, Antony Dovgal

Rasmus Lerdorf

18 years ago
Antony Dovgal wrote:
> On 08.09.2007 21:31, Rasmus Lerdorf wrote: >> Facebook is quite active in APC development. > > I'm glad for APC, but this is a separate project (or a sub-project, whatever). > >> And yes, if it turns out to be 10% slower, a lot of sites are going to want to run without it. > > Right, but saying that we should be especially careful because this patch may affect company XXX makes no sense to me. > Either this company is interested in this patch and helps us to make it better, or we follow the usual development process > (i.e. try to make it as better as we can and release when it's ready, but we don't test it for years to stabilize it). > >> For scripts that aren't very complex and don't run for a long time, GC >> simply isn't a very interesting feature. > > Well, for not very complex scripts most of the engine improvements are not very interesting.
Sure, but most of them also aren't detrimental to the short and fast scripts that make up most efficient sites. Like Stas says, try to avoid penalties for people who don't use a particular new feature. This falls under that same category. -Rasmus

Cristian Rodriguez

18 years ago
On 9/8/07, Rasmus Lerdorf <rasmus@lerdorf.com> wrote: And yes, if it turns out
> to be 10% slower, a lot of sites are going to want to run without it.
Sad, I see you have lost the notion about who are the vast mayority of your users. Facebook or any top site that requires extreme performance represents a very small (less than 10%) of your real users. Instead of discussing about speed, I think we should talk about proper behaviuor first, premature optimization is an horrible bad thing.
-- http://www.kissofjudas.net/

David

18 years ago
It's been about 3 days since the last comment on the patch itself on this thread and nearly a week since the patch was submitted for comment. Until a patch is committed, I can't publish a patch for the GC, and until that's published, there's really nothing to debate about yet. :) I can't commit any version of the patch as no one has responded to my CVS request. I'm not sure what the standard protocol is in this case, but perhaps someone with the necessary karma could take a vote on which version of the patch to commit and then act accordingly? David

Brian Shire

18 years ago
On Sep 8, 2007, at 9:31 AM, Antony Dovgal wrote:
> > If these companies really do worry about this %, why don't they > participate in development and/or testing? > I don't recall seeing any contributions or even feedback from > Facebook or alike, do you? > > If they wait for a release to test and complain, then I couldn't > care less - it's just a common > development process, bugs come and bugs go, I see no reasons why we > should make an exception for > a company that doesn't give a damn about us.
I realize that this conversation isn't really about Facebook, but while it's been brought up I'd like to step in and say that Facebook is *very* interested in helping with PHP development and other open source projects. We are always trying to run the latest CVS or RC versions to make sure we don't find any problems or performance losses (although we do occasionally fall behind which is unfortunate for us). If there are any new patches that the developers think we could help test out for performance and stability I'm very happy to do so. There have been a handful small contributions to APC, PHP, and others, and I'm excited that we get to work on these projects and I hope myself and others at Facebook will be able to continue to make useful contributions. I'm also grateful for the contributions others have made here, and I hope Facebook and myself can establish itself as an entity dedicated to giving back as well. If you have other questions or suggestions please feel free to email me, I know I've limited my posts here so as not to add anything unnecessary but we are here and ready to help. Thanks, -shire Facebook Inc. shire@facebook.com

David

18 years ago
I'm not really sure what this big debate about. I'm sure that everyone agrees (not least of all me!) that the GC patch should be thoroughly tested for performance and stability before being released to the public. I mean, something that affects the life-cycle of every zval definitely has to be. However, many people are hypothesizing that GC might be slow, as much as 10% even. I know this is just being used as an argument for more testing (which I totally agree with) but I feel like it's been discussed so much that I need to step up and defend my baby. ;) About 25% of my time was spent actually getting the GC to work right. 75% of it was trying to make it work fast. I'm definitely not naive about PHP's use in enterprise applications where a few % really does matter and I've been on the list long enough to know that a lot of people here care about those few % as well. :) My basic principle was that "People who don't need or don't want GC should not see any performance impact," and I believe I have achieved that. (There was also an addedum "if necessary, at the expense of people who do need or want GC" but shhh ;)) There are various housekeeping tasks that must be written into the engine that can't really be configured out after compile. I tried to keep the impact of those as low as possible. I went as far as using callgrind to make sure L1 cache misses were kept low. As it stands right now, I don't see any significant performance impact on bench.php (which I think would be a good test of those changes), not even 0.1%. Then there are greater pieces of maintenance that must be performed to keep track of possible gc targets. Those can be configured out using php.ini or even user level functions. Users can safely control at the script level whether they want GC or not. However, those aren't even very costly since they also don't make a (measurable on my setup) difference on performance but the impact of these functions are more script dependent and will require more testing. However, in most cases, you should be able to leave GC enabled and not see any performance impact because the part the impacts performance significantly: the collection, only happens when the number of collectable cycles you have is above a (very large) threshold. That's not going to happen during most scripts (where memory consumption is low and run time is short). GC is not fully ready yet. It really MUST be tested on a wider scale, and on more real-world applications, and I think everyone agrees. Without testing, It's even too early to tell how much more work it needs yet. I spent an enormous amount of time and effort trying to make GC as fast and painless as possible and I'm extremely eager for others to begin testing it to see if the time I invested has paid off. David On 9/9/07, shire <shire@facebook.com> wrote: