wp-cli / profile-command

Quickly identify what's slow with WordPress
MIT License
271 stars 29 forks source link

Support running profile against other WP CLI commands #138

Closed joehoyle closed 7 years ago

joehoyle commented 7 years ago

It would be pretty sweet if I could do:

wp --profile media regenerate-thumbnails and it show me the profile for that execution after it's done.

danielbachhuber commented 7 years ago

It would be pretty sweet if I could do:

As it turns out, you can!

  1. Create a PHP file to execute
$ cat media-regenerate.php
<?php
WP_CLI::runcommand( 'media regenerate --yes', array( 'launch' => false ) );
  1. Profile PHP file execution
$ wp profile eval-file media-regenerate.php
Found 1 image to regenerate.
1/1 Regenerated thumbnails for "download" (ID 7).
Success: Regenerated 1 of 1 images.
+---------+------------+-------------+-------------+------------+--------------+--------------+---------------+
| time    | query_time | query_count | cache_ratio | cache_hits | cache_misses | request_time | request_count |
+---------+------------+-------------+-------------+------------+--------------+--------------+---------------+
| 0.9374s | 0.0284s    | 6           | 88.68%      | 47         | 6            | 0s           | 0             |
+---------+------------+-------------+-------------+------------+--------------+--------------+---------------+
schlessera commented 7 years ago

I don't think the above result is correct. It doesn't show any internal profiling data (like the query count and time), it seems to only do what a basic time would do: diff the start and end times.

danielbachhuber commented 7 years ago

Oops. I forgot to keep the current process.

I've updated my comment to include:

WP_CLI::runcommand( 'media regenerate --yes', array( 'launch' => false ) );
schlessera commented 7 years ago

Hehe, this ties neatly into https://github.com/wp-cli/wp-cli/issues/3890 : It shouldn't be as easy to have completely different behavior (resulting in a bug) just because you forgot an obscure array argument.

danielbachhuber commented 7 years ago

It shouldn't be as easy to have completely different behavior (resulting in a bug) just because you forgot an obscure array argument.

The behavioral difference occurs because WP_CLI::runcommand()'s default behavior is to launch a new process. In order to track queries, etc., wp profile needs access to the $wpdb variable of the process, which it doesn't have when a new process is launched.

Technically speaking, the original result was correct, because no queries occurred in the current process.

Could you explain how https://github.com/wp-cli/wp-cli/issues/3890 would've caught this? I think it's reasonable to have default behavior for methods, and optional arguments to change the behavior.

schlessera commented 7 years ago

I think it should be obvious from either the method or the arguments what the expected behavior is. So, in the above example, a very simple disambiguation would be to either require the 'launch' argument, or to have different methods for these very different behaviors, like run_command_in_same_process and run_command_in_new_process (I know, not the catchiest method names).

The point is that the intent should be clear and unambiguous. Providing default fallback values is nice for situations where it's mostly just about convenience. But in this particular case, runcommand is actually a facade to an entire category of methods, and they are chosen based on array arguments, which are neither validated nor checked in any other way. The default fallback values actually choose which specific type of functionality to run whenever you didn't provide overrides.

danielbachhuber commented 7 years ago

I've left a comment on the linked issue.