khromov / wp-performance-profiler

A fork of WP Performance Profiler by interconnect/it in accordance with the GPL.
45 stars 10 forks source link

PHP7 compatibility #1

Open mdorchain opened 8 years ago

mdorchain commented 8 years ago

Hi Stanislav, WP Performance Profiler used to have serious issues running on PHP7. Is this something you plan to look into ?

khromov commented 8 years ago

@mdorchain This fork was started because I was getting some really strange and inaccurate results in the functions breakdown and am trying to find the root cause of that. If I manage to track the issue down, PHP 7 support is probably not that hard to add. Is there any specific problem you are seeing?

mdorchain commented 8 years ago

I couldn't see errors but the issue was obvious. WP Performance Profiler wouldn't see any other plugin than itself on PHP7. The interesting thing is P3 Profiler had the same exact behavior.

The reply from Interconnectit was

"The semantics of declare() / ticks appears to have changed in PHP7. This causes the performance profiler only to benchmark itself. This is undocumented and therefore perhaps is a bug in PHP, but in any case we will add a notice about PHP7-compatibility."

As a result the compatibility is up to PHP 5.6.17.

stodorovic commented 8 years ago

Hi Michael , Stanislav

It's same story with all plugins, ( Interconnectit, Godady, ...).

http://php.net/manual/en/migration70.incompatible.php - PHP 7.0 changed behavior. Reason for all troubles is function debug_backtrace() which all plugins use for checking.

Functions inspecting arguments report the current parameter _func_get_arg(), func_get_args(), debugbacktrace() and exception backtraces will no longer report the original value that was passed to a parameter, but will instead provide the current value (which might have been modified).

function do_tick() { // Get the current trace $trace = debug_backtrace();

PHP 5.6 shows backtraces from function which is interrupted, but PHP 7.0 shows backtraces for do_tick function (and it always points to same file - plugin and you don't see any other plugin).

Only old wp-profiler works, but it needs "coding" for each run. It doesn't use register_tick_function, it use 'all' hook in WP. I'm trying to make something, but it's very hard, basically we need to create totally new logic.

Sasa

khromov commented 8 years ago

@stodorovic I've noticed this as well. Don't currently have time to work on this but if you get something going feel free to submit a PR and I'll try to help out.

davidfavor commented 8 years ago

Sasa, please provide a link to the "old wp-profiler" as there are several code systems using this same name + unsure which is the correct one to try.

Thanks.

stodorovic commented 8 years ago

Hi David,

Sorry for delay. I'm working on something else (opposite of profiler). Basically, one of version of profiler is there: https://github.com/aaemnnosttv/wordpress-tests-core/blob/master/wp-profiler.php (But you can't use it without injection of PHP code or ...).

Also, I was trying to develop something. I found some examples into:

https://runcommand.io/wp/hook/ https://runcommand.io/wp/profile/

https://wordpress.org/plugins/hook-timer/ (It doesn't work with some plugins and it's just an idea).

I was able to made something between runcommand profile and hook-timer. Basically, it shows exec time, query time, number of triggers for each action/filter. Also, WP will change actions/filter in new WP 4.7:

https://make.wordpress.org/core/2016/09/08/wp_hook-next-generation-actions-and-filters/

So, I decided to wait a bit with this project. Also, my current version is enough for my internal purpose, but it needs more testing, checking, etc,....

I hope that's helpful.

danielbachhuber commented 8 years ago

"The semantics of declare() / ticks appears to have changed in PHP7. This causes the performance profiler only to benchmark itself. This is undocumented and therefore perhaps is a bug in PHP, but in any case we will add a notice about PHP7-compatibility."

Here's the bug on PHP.net: https://bugs.php.net/bug.php?id=72966

jb510 commented 7 years ago

I swear I posted this somewhere... but the resolution of that PHP Bug seems to be "this was an intended change in the behavior of ticks / won't fix"

So the question is is there another way to get WPP (which I love but is a giant mystery to me) to function in PHP7/7.1?

jb510 commented 6 years ago

I really love this plugin. Is there any hope of it getting rebuilt to work on PHP7+?

Following the PHP tickets on this, https://bugs.php.net/bug.php?id=71448, it looks like https://wiki.php.net/rfc/async_signals could be used in place of ticks. Sounds so simple, but way beyond my understanding of the internals of PHP or this magical plugin.

khromov commented 6 years ago

@jb510 Good timing, I was just looking at another approach to this a couple of days ago, which includes a working proof of concept: https://stackoverflow.com/a/44616349

Couldn't make it work though, it seems more functions in the stream wrapper need to be implemented, but it could be a viable approach

I'll have a look at the function you posted.

In other news, I'll be pushing a change shortly that fixes a bug with incorrect memory usage displayed in the stats.

khromov commented 6 years ago

I tested the stream wrapper approach. Good news: It does seem to theoretically be able to accomplish what we need (ie. prepend declare(ticks=1) after the starting <?php tag on every included file. Bad news: PHP complains that too much data is returned and breaks after a while, so it's not functional.

If we can fix the issue with too much data being returned, we should be very close to a solution.

Experimental PR here: https://github.com/khromov/wp-performance-profiler/pull/2

kel commented 6 years ago

Hello, I received an email from Interconnect today saying the plugin now works with PHP 7.2.x. Did PHP revert the previous changes? I see the ticket still shows won't fix.

This was the message in the email:

WP Performance Profiler works in PHP 7.2.x - this version removes the warning for PHP 7.2.x, but is otherwise unchanged from the previous release.

Please note that due to a regression in PHP 7.0.x and PHP 7.1.x, the plugin will not work correctly on those versions, and the warning message will remain in force.

khromov commented 6 years ago

@kel I can't find anything in the changelog, migration guide or bug tracker to suggest any change has been done in PHP 7.2 pertaining to the handling of ticks. That does not mean nothing was done, just that it's not immediately obvious. I don't follow the Interconnect/IT mailing list anymore, do you happen to know if they have updated the plugin recently? This fork was forked from the 0.3 version of their plugin. If you have the email text in its entirety, that'd also be appreciated. But just by the sounds of it, the plugin might work again in 7.2 by itself.

kel commented 6 years ago

@khromov What I quoted was the full content of the email minus a download link.

I can confirm that it works in 7.2 and this version is 0.3.3. Am I able to send it to you to look at? I see it's GPLv2 but I don't want to violate the license.

khromov commented 6 years ago

@kel It's fine to share it according to the GPL license. The easiest thing would be if you could make a PR for that version to this repository, and we could then merge it cleanly with the existing code changes.

kel commented 6 years ago

@khromov I created a pull request. I'm not a git master so let me know if I need to do something different.

khromov commented 6 years ago

@kel Thanks! The PR looks fine, just have to merge it and test how it runs on PHP 7.2!

JohnMerrick commented 6 years ago

Still doesnt work under PHP 7.2, you only see the perfomance of the core, you don't get the breakdown for each plugin or for the theme, plugins and database