-
Notifications
You must be signed in to change notification settings - Fork 30
Introduce wp profile queries command
#207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
wp profile queries command
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
|
@copilot There's lots of PHP warnings "Warning: A non-numeric value encountered in src/Formatter.php on line 134" in tests for this new code. Please fix the root cause. |
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Fixed by adding non-numeric field detection in Formatter.php. The code now skips string fields ('query', 'caller', 'hook', 'callback') when calculating totals and includes an is_numeric() check before summing values to prevent the warnings. (commit 28e1e4d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| foreach ( $logger->query_indices as $query_index ) { | ||
| if ( ! isset( $query_map[ $query_index ] ) ) { | ||
| $query_map[ $query_index ] = array( | ||
| 'hook' => isset( $logger->hook ) ? $logger->hook : null, | ||
| 'callback' => isset( $logger->callback ) ? $logger->callback : null, | ||
| ); | ||
| } | ||
| } |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Potential issue with query map overwriting. When multiple loggers track the same query index (e.g., nested hooks), the check ! isset( $query_map[ $query_index ] ) at line 602 prevents overwriting. This means the first logger's hook/callback will be used, but in nested scenarios, this might not be the most specific or accurate mapping. Consider whether the first-logger-wins approach is the intended behavior, or if you need to track multiple hooks/callbacks per query.
| $loggers = $profiler->get_loggers(); | ||
| foreach ( $loggers as $logger ) { | ||
| // Skip if filtering by callback and this isn't the right one | ||
| if ( $callback && isset( $logger->callback ) ) { | ||
| // Normalize callback for comparison | ||
| $normalized_callback = str_replace( array( '->', '::' ), '', (string) $logger->callback ); | ||
| $normalized_filter = str_replace( array( '->', '::' ), '', $callback ); | ||
| if ( false === stripos( $normalized_callback, $normalized_filter ) ) { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Skip if filtering by hook and this isn't the right one | ||
| if ( $hook && isset( $logger->hook ) && $logger->hook !== $hook ) { | ||
| continue; | ||
| } | ||
|
|
||
| // Get the query indices for this logger | ||
| if ( isset( $logger->query_indices ) && ! empty( $logger->query_indices ) ) { | ||
| foreach ( $logger->query_indices as $query_index ) { | ||
| if ( ! isset( $query_map[ $query_index ] ) ) { | ||
| $query_map[ $query_index ] = array( | ||
| 'hook' => isset( $logger->hook ) ? $logger->hook : null, | ||
| 'callback' => isset( $logger->callback ) ? $logger->callback : null, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Get all queries | ||
| $queries = array(); | ||
| if ( ! empty( $wpdb->queries ) ) { | ||
| foreach ( $wpdb->queries as $index => $query_data ) { | ||
| // If filtering by hook/callback, only include queries in the map | ||
| if ( ( $hook || $callback ) && ! isset( $query_map[ $index ] ) ) { | ||
| continue; | ||
| } | ||
|
|
||
| $query_obj = new QueryLogger( | ||
| $query_data[0], // SQL query | ||
| $query_data[1], // Time | ||
| isset( $query_data[2] ) ? $query_data[2] : '', // Caller | ||
| isset( $query_map[ $index ]['hook'] ) ? $query_map[ $index ]['hook'] : null, | ||
| isset( $query_map[ $index ]['callback'] ) ? $query_map[ $index ]['callback'] : null | ||
| ); | ||
| $queries[] = $query_obj; | ||
| } |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Performance consideration: The nested loop structure (lines 583-610 and 616-630) could be inefficient for sites with many queries and hooks. For N loggers and M queries, this is O(N*Q + M) where Q is queries per logger. If $wpdb->queries contains thousands of entries, this might be slow. Consider optimizing by building the query map in a single pass if performance becomes an issue, though the current implementation is clear and correct for typical use cases.
| $this->query_indices[] = $i; | ||
| } |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The $query_indices array is not reset in the stop() method, unlike other tracking properties (e.g., $query_offset is set to null on line 111). If a Logger instance is reused by calling start() and stop() multiple times, the $query_indices array will accumulate indices across multiple runs. Consider resetting $this->query_indices = array(); in the stop() method or at the start of start() method to ensure clean state for reused loggers, or document that loggers should not be reused.
src/Command.php
Outdated
| if ( $hook ) { | ||
| $type = 'hook'; | ||
| $focus = $hook; | ||
| } elseif ( $callback ) { | ||
| $type = 'hook'; | ||
| $focus = true; // Profile all hooks to find the specific callback | ||
| } |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When both --hook and --callback are provided, the profiler setup logic only uses the hook value. The condition on line 571 (elseif ( $callback )) will never be reached when both are present. This means the profiler won't track all hooks to find the specific callback, which could lead to incomplete filtering. Consider handling the case where both are provided explicitly, or adjust the logic to ensure both parameters are considered properly.
…indices Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Adds a new
wp profile queriescommand to display database queries executed during a WordPress request with timing information and optional filtering by hook or callback.Changes
queries()method that runs WordPress profiling, collects queries from$wpdb->queries, and outputs them with configurable fields and filtering. Includes proper handling for both--hookand--callbackparameters used together, and explicit filtering logic to skip loggers without callbacks when callback filtering is requested.query_indicesarray duringstop(), enabling accurate query-to-hook/callback mapping. Thequery_indicesarray is now properly reset instop()method to ensure clean state for reused loggers.features/profile-queries.featurewith scenarios for basic listing, filtering, and output formats. Tests include custom mu-plugins that execute specific database queries to verify functionality.Usage
Filtering works by tracking which query indices each Logger executes (via
query_offsetrange), then matching those indices to hook/callback names when filtering is requested. When both--hookand--callbackare provided, the command profiles all hooks and then filters to show only queries that match both criteria.wp profile queries#130Original prompt
wp profile queries#130✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.