Add timing to garbage collection when system supports gettimeofday().

[beberlei]

This allows Profilers that hook into the zend_execute stack to gather information about the total time of garbage collection, or depending on the implementation even time of GC in a single function call.

Currently Profilers cannot get this information and therefore the resulting data can be very misleading if the GC is triggered accidently during various function calls.

Sample usage in Qafoo's PHP Profiler extension: https://github.com/QafooLabs/php-profiler-extension/pull/6/files

The inclusion of this change is debatable, first its really only necessary for Profilers, adding overhead of the gettimeofday call in any case. However given that GC is triggered in very rare cases this might be considered OK. Composer for multi second long calls only triggers this cleanup 174 times (Reference: composer/composer#3482 (comment))

The other approach could be to make gc_collect_cycles hookable by changing it to a function pointer.

[LawnGnome]

I threw a quick and dirty patch together at https://github.com/LawnGnome/php-src/compare/gc-fn-pointer to implement this by adding a function pointer, as we do with the execute hook.

Testing Zend/bench.php turned out to be useless, as we never collect cycles there. I wrote a quick test script that does exercise the GC, and uploaded it to a repository (more tests welcome!): https://github.com/LawnGnome/php-gc-benchmark

There's no discernable difference between the approaches on any platform I've tried. They're close enough to identical, even on synthetic benchmarks, which suggests that there's unlikely to be any noticeable overhead implementing some sort of instrumentation.

I do still prefer the function pointer approach: it's more flexible for extensions implementing profiling (since they can attach the GC cost to the profile where it actually happened), and avoids the hit of gettimeofday on paravirtualised Linux systems where that potentially incurs a significant cost, albeit at the cost of making a profiler that only reports the headline number more complex to implement.

What do you think? Are there more tests you'd want to try? Concerns over the function pointer approach?

[beberlei]

@LawnGnome I like the function pointer approach more as well, the patch looks just much better and avoids cluttering the default code base that doesn't need timing.

One problem though is that you have find out if the GC actually run implicitly, by testing for GC(runs) before and after calling the original pointer, but I guess its possible to do so no problem.

This will only land in 5.7 (if there is one) and 7.0 anyways, so maybe it would even be better to split 'gc_collect_cycles' into two functions gc_check_collect_cycles' and that one triggersgc_collect_cycles`.

[LawnGnome]

What do you define as "running" in this case? gc_collect_cycles returns the number of zvals that were GCed, so checking if that is 0 would suffice, I'd think.

[beberlei]

@LawnGnome Well there could be the case where even when gc collect runs, it still does not cleanup a single zval. So you cant detect from the return value alone. But I guess it doesn't matter anyways. Instrumentation would find out that time was spent there, regardless of how mny objects where cleaned up.

[LawnGnome]

@beberlei Yeah, I see what you mean now. I think that's an obscure enough distinction that it's not worth adding an extra function call to something that needs to be fast, particularly since it's discoverable by examining the GC globals, as you say. (I also think the common case is just going to be looking at the time spent in zend_gc_collect_cycles, in which case it doesn't matter.)

Assuming you're OK with that, I guess the next step is an RFC. (It's debatable if this really needs one, but we might as well make it explicit.)

Do you want to write that, or shall I? (I'm happy to, but since you got the ball rolling, I'm also happy to defer!)

[beberlei]

@LawnGnome I'd really like to write the draft, will link it to you here. Thanks

[beberlei]

@LawnGnome I came up with https://wiki.php.net/rfc/gc_fn_pointer - Can you proove read and either add or point me towards things missing? I didn't put your name on there yet to wait for your approval, but you can just add it if everything is alright. It is late at n8 here in Europe, I will send a mail to the mailing list tomorrow after your review.

[LawnGnome]

@beberlei Nice job! You might want to note that it has no measurable impact on performance, and I'd like to make it clear that this would be proposed for an eventual PHP 5.7 as well (speaking of RFCs I need to write).

Otherwise, I think this is good to go. Thanks for writing the RFC. :)