feat: Enhanced WordPress hook profiling with advanced filtering - Plugin performance analysis UI #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…gin performance analysis UI - Implemented comprehensive hook profiler interface with multi-tab navigation - Added plugin loading analysis with timing breakdowns by plugin type (sunrise, mu-plugins, network, regular) - Enhanced callback wrapper with detailed timing aggregation and plugin detection - Added advanced filtering capabilities for hooks, plugins, and callbacks with real-time search - Introduced performance visualization with color-coded timing indicators - Integrated sorting and grouping functionality for better data analysis
WalkthroughUpdates profiler UI logic to prefer plugin_name, handle NaN-safe times, compute and sort by total_time, and introduce refined per-hook filtering/search (by plugin and search term) with visibility counts. PHP adds plugin_name to callback aggregates. No public API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Profiler UI
participant Data as Callback Groups
participant Filter as filterHooksBySearch / plugin-filter
participant Render as Renderer
User->>UI: Set search term / plugin filter
UI->>Filter: Apply search and/or plugin filter
alt Plugin filter active
Filter->>Data: Filter callbacks within each hook
Filter-->>UI: Matching hooks + visible/total counts
else Search only
Filter->>Data: Search by hook/callback/meta
Filter-->>UI: Matching hooks (preserve callback visibility)
end
UI->>Data: Compute total_time per group (safe defaults)
UI->>Data: Sort groups by total_time
UI->>Render: Render headers (plugin_name, counts, times)
Render-->>User: Updated table (NaN-safe times)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/class-callback-wrapper.php (1)
60-79: Bug: totals updated even when $eta is NaN/invalid.Plugin totals are incremented outside the finite/positive guard, which can propagate NaN or negative values into profileData.plugins. Move the totals block under the same validation.
Apply this diff:
- // Update plugin totals - $plugin_key = $plugin_info['plugin']; - if (!isset($this->engine->timing_data[$plugin_key])) { + // Update plugin totals (guarded) + if (is_finite($eta) && $eta >= 0) { + $plugin_key = $plugin_info['plugin']; + if (!isset($this->engine->timing_data[$plugin_key])) { $this->engine->timing_data[$plugin_key] = [ 'total_time' => 0, 'hook_count' => 0, 'callback_count' => 0, 'hooks' => [], 'plugin_name' => $plugin_info['plugin_name'], 'plugin_file' => $plugin_info['plugin_file'] ]; - } - - $this->engine->timing_data[$plugin_key]['total_time'] += $eta; - $this->engine->timing_data[$plugin_key]['callback_count']++; + } + $this->engine->timing_data[$plugin_key]['total_time'] += $eta; + $this->engine->timing_data[$plugin_key]['callback_count']++; if (!in_array($this->hook_name, $this->engine->timing_data[$plugin_key]['hooks'])) { $this->engine->timing_data[$plugin_key]['hooks'][] = $this->hook_name; $this->engine->timing_data[$plugin_key]['hook_count']++; } + }
🧹 Nitpick comments (6)
inc/class-callback-wrapper.php (1)
48-58: PHP compatibility: avoid dependency on is_finite availability.On older environments, is_finite may be unavailable. Use a fallback check to keep behavior consistent.
Apply this diff:
- if (is_finite($eta) && $eta >= 0) { + $eta_is_finite = function_exists('is_finite') ? is_finite($eta) : (!is_nan($eta) && !is_infinite($eta)); + if ($eta_is_finite && $eta >= 0) {assets/profiler.js (5)
190-201: Make plugin filtering robust: add data attribute for exact matching.Relying on meta text parsing is brittle and not i18n-safe. Store plugin on the item.
Apply this diff:
- <div class="wp-hook-profiler-callback-item"> + <div class="wp-hook-profiler-callback-item" data-plugin="${escapeHtml(callback.plugin_name || callback.plugin)}">And adjust the filter (see below).
174-181: Nit: escape hookName in data-hook attribute.Prevents malformed attributes if hook names contain special chars.
Apply this diff:
- <div class="wp-hook-profiler-hook-group" data-hook="${hookName}"> + <div class="wp-hook-profiler-hook-group" data-hook="${escapeHtml(hookName)}">
313-332: Use data-plugin instead of parsing text for filtering.This avoids false positives and supports future localization.
Apply this diff:
- const callbackItems = $(this).find('.wp-hook-profiler-callback-item'); - callbackItems.each(function() { - const callbackMeta = $(this).find('.wp-hook-profiler-callback-meta'); - const metaText = callbackMeta.text(); - // Check if this callback matches the plugin filter - const matchesPlugin = metaText.includes(`Plugin: ${pluginFilter}`) || - metaText.toLowerCase().includes(`plugin: ${pluginFilter.toLowerCase()}`) || - metaText.toLowerCase().includes(pluginFilter.toLowerCase()); - if (matchesPlugin) { + const callbackItems = $(this).find('.wp-hook-profiler-callback-item'); + callbackItems.each(function() { + const itemPlugin = ($(this).data('plugin') || '').toString(); + const matchesPlugin = itemPlugin.toLowerCase() === pluginFilter.toLowerCase(); + if (matchesPlugin) { hookHasMatchingCallbacks = true; visibleCallbackCount++; $(this).show(); } else { $(this).hide(); } });
355-362: Remove commented debug logs.Dead comments add noise. Consider deleting or gating behind a verbose flag.
98-100: NaN-safe total time in summary.Guard against undefined/NaN to avoid showing “NaN”.
Apply this diff:
- $('#wp-hook-profiler-total-time').text((profileData.total_execution_time).toFixed(2)); + const totalTime = Number(profileData.total_execution_time) || 0; + $('#wp-hook-profiler-total-time').text(totalTime.toFixed(2));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
assets/profiler.js(6 hunks)inc/class-callback-wrapper.php(1 hunks)
🔇 Additional comments (4)
inc/class-callback-wrapper.php (1)
31-31: Good addition: expose human-friendly plugin name.Including plugin_name in aggregates aligns backend with the UI and filtering needs.
assets/profiler.js (3)
151-152: UI correctness: prefer plugin_name with safe fallback.Good fallback to callback.plugin when plugin_name is absent.
299-305: Nice: split search-only path for performance.The early return reduces DOM work when only searching.
408-409: Sorting by total_time makes sense.Aligns sorting with displayed metrics and avoids undefined penalties via fallback 0.
Comment on lines
+170
to
178
| const totalTime = callbacks.reduce((sum, cb) => sum + (cb.total_time || 0), 0) * 1000; | ||
| const timeClass = getTimeColorClass(totalTime); | ||
|
|
||
| const hookGroup = $(` | ||
| <div class="wp-hook-profiler-hook-group" data-hook="${hookName}"> | ||
| <div class="wp-hook-profiler-hook-header"> | ||
| ${escapeHtml(hookName)} | ||
| <span class="${timeClass}" style="float: right;">${totalTime.toFixed(3)}ms</span> | ||
| <span class="${timeClass}" style="float: right;">${isNaN(totalTime) ? '0.000' : totalTime.toFixed(3)}ms</span> | ||
| </div> |
There was a problem hiding this comment.
Critical: unit error inflates hook total time by 1000x.
callback.total_time is already in milliseconds; multiplying by 1000 mislabels values and color classes.
Apply this diff:
- const totalTime = callbacks.reduce((sum, cb) => sum + (cb.total_time || 0), 0) * 1000;
+ const totalTime = callbacks.reduce((sum, cb) => sum + (cb.total_time || 0), 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const totalTime = callbacks.reduce((sum, cb) => sum + (cb.total_time || 0), 0) * 1000; | |
| const timeClass = getTimeColorClass(totalTime); | |
| const hookGroup = $(` | |
| <div class="wp-hook-profiler-hook-group" data-hook="${hookName}"> | |
| <div class="wp-hook-profiler-hook-header"> | |
| ${escapeHtml(hookName)} | |
| <span class="${timeClass}" style="float: right;">${totalTime.toFixed(3)}ms</span> | |
| <span class="${timeClass}" style="float: right;">${isNaN(totalTime) ? '0.000' : totalTime.toFixed(3)}ms</span> | |
| </div> | |
| const totalTime = callbacks.reduce((sum, cb) => sum + (cb.total_time || 0), 0); | |
| const timeClass = getTimeColorClass(totalTime); | |
| const hookGroup = $(` | |
| <div class="wp-hook-profiler-hook-group" data-hook="${hookName}"> | |
| <div class="wp-hook-profiler-hook-header"> | |
| ${escapeHtml(hookName)} | |
| <span class="${timeClass}" style="float: right;">${isNaN(totalTime) ? '0.000' : totalTime.toFixed(3)}ms</span> | |
| </div> |
🤖 Prompt for AI Agents
In assets/profiler.js around lines 170 to 178, the code multiplies
callback.total_time by 1000 but callback.total_time is already in milliseconds,
inflating displayed totals and affecting color classification; remove the * 1000
multiplier so totalTime is computed as const totalTime = callbacks.reduce((sum,
cb) => sum + (cb.total_time || 0), 0); keep the subsequent
getTimeColorClass(totalTime) and the isNaN/ toFixed formatting unchanged so
values and classes reflect true milliseconds.
Comment on lines
+310
to
+353
| let hookHasMatchingCallbacks = !pluginFilter; | ||
| let visibleCallbackCount = 0; | ||
|
|
||
| if (pluginFilter) { | ||
| const hasMatchingPlugin = $(this).find('.wp-hook-profiler-callback-meta') | ||
| .toArray() | ||
| .some(el => $(el).text().includes(`Plugin: ${pluginFilter}`)); | ||
| matchesPlugin = hasMatchingPlugin; | ||
| // Filter individual callbacks within this hook group | ||
| const callbackItems = $(this).find('.wp-hook-profiler-callback-item'); | ||
| callbackItems.each(function() { | ||
| const callbackMeta = $(this).find('.wp-hook-profiler-callback-meta'); | ||
| const metaText = callbackMeta.text(); | ||
|
|
||
| // Check if this callback matches the plugin filter | ||
| const matchesPlugin = metaText.includes(`Plugin: ${pluginFilter}`) || | ||
| metaText.toLowerCase().includes(`plugin: ${pluginFilter.toLowerCase()}`) || | ||
| metaText.toLowerCase().includes(pluginFilter.toLowerCase()); | ||
|
|
||
| if (matchesPlugin) { | ||
| hookHasMatchingCallbacks = true; | ||
| visibleCallbackCount++; | ||
| $(this).show(); | ||
| } else { | ||
| $(this).hide(); | ||
| } | ||
| }); | ||
| } else { | ||
| // No plugin filter - show all callbacks | ||
| $(this).find('.wp-hook-profiler-callback-item').show(); | ||
| hookHasMatchingCallbacks = true; | ||
| visibleCallbackCount = $(this).find('.wp-hook-profiler-callback-item').length; | ||
| } | ||
|
|
||
| // Show/hide the entire hook group based on whether it has matching callbacks | ||
| const shouldShowHook = matchesSearch && hookHasMatchingCallbacks && visibleCallbackCount > 0; | ||
| $(this).toggle(shouldShowHook); | ||
|
|
||
| // Update hook header to reflect filtered callback count | ||
| if (shouldShowHook && pluginFilter) { | ||
| const totalCallbacks = $(this).find('.wp-hook-profiler-callback-item').length; | ||
| const hookHeader = $(this).find('.wp-hook-profiler-hook-header'); | ||
| const originalText = hookHeader.text().replace(/\(\d+\/\d+\)/, '').trim(); | ||
| if (visibleCallbackCount !== totalCallbacks) { | ||
| hookHeader.html(`${originalText} <small>(${visibleCallbackCount}/${totalCallbacks} callbacks)</small>`); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Bug: header rewrite drops the right-aligned time span.
Using hookHeader.text() then .html() nukes existing child nodes (time badge) and may duplicate labels on subsequent filters.
Apply this diff to preserve the time span and only toggle a dedicated count node:
- // Update hook header to reflect filtered callback count
- if (shouldShowHook && pluginFilter) {
- const totalCallbacks = $(this).find('.wp-hook-profiler-callback-item').length;
- const hookHeader = $(this).find('.wp-hook-profiler-hook-header');
- const originalText = hookHeader.text().replace(/\(\d+\/\d+\)/, '').trim();
- if (visibleCallbackCount !== totalCallbacks) {
- hookHeader.html(`${originalText} <small>(${visibleCallbackCount}/${totalCallbacks} callbacks)</small>`);
- }
- }
+ // Update hook header to reflect filtered callback count (preserve time span)
+ if (shouldShowHook && pluginFilter) {
+ const totalCallbacks = $(this).find('.wp-hook-profiler-callback-item').length;
+ const hookHeader = $(this).find('.wp-hook-profiler-hook-header');
+ let countEl = hookHeader.find('.wp-hook-profiler-hook-count');
+ if (!countEl.length) {
+ countEl = $('<small class="wp-hook-profiler-hook-count"></small>').appendTo(hookHeader);
+ }
+ if (visibleCallbackCount !== totalCallbacks) {
+ countEl.text(` (${visibleCallbackCount}/${totalCallbacks} callbacks)`);
+ } else {
+ countEl.remove();
+ }
+ } else {
+ $(this).find('.wp-hook-profiler-hook-header .wp-hook-profiler-hook-count').remove();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let hookHasMatchingCallbacks = !pluginFilter; | |
| let visibleCallbackCount = 0; | |
| if (pluginFilter) { | |
| const hasMatchingPlugin = $(this).find('.wp-hook-profiler-callback-meta') | |
| .toArray() | |
| .some(el => $(el).text().includes(`Plugin: ${pluginFilter}`)); | |
| matchesPlugin = hasMatchingPlugin; | |
| // Filter individual callbacks within this hook group | |
| const callbackItems = $(this).find('.wp-hook-profiler-callback-item'); | |
| callbackItems.each(function() { | |
| const callbackMeta = $(this).find('.wp-hook-profiler-callback-meta'); | |
| const metaText = callbackMeta.text(); | |
| // Check if this callback matches the plugin filter | |
| const matchesPlugin = metaText.includes(`Plugin: ${pluginFilter}`) || | |
| metaText.toLowerCase().includes(`plugin: ${pluginFilter.toLowerCase()}`) || | |
| metaText.toLowerCase().includes(pluginFilter.toLowerCase()); | |
| if (matchesPlugin) { | |
| hookHasMatchingCallbacks = true; | |
| visibleCallbackCount++; | |
| $(this).show(); | |
| } else { | |
| $(this).hide(); | |
| } | |
| }); | |
| } else { | |
| // No plugin filter - show all callbacks | |
| $(this).find('.wp-hook-profiler-callback-item').show(); | |
| hookHasMatchingCallbacks = true; | |
| visibleCallbackCount = $(this).find('.wp-hook-profiler-callback-item').length; | |
| } | |
| // Show/hide the entire hook group based on whether it has matching callbacks | |
| const shouldShowHook = matchesSearch && hookHasMatchingCallbacks && visibleCallbackCount > 0; | |
| $(this).toggle(shouldShowHook); | |
| // Update hook header to reflect filtered callback count | |
| if (shouldShowHook && pluginFilter) { | |
| const totalCallbacks = $(this).find('.wp-hook-profiler-callback-item').length; | |
| const hookHeader = $(this).find('.wp-hook-profiler-hook-header'); | |
| const originalText = hookHeader.text().replace(/\(\d+\/\d+\)/, '').trim(); | |
| if (visibleCallbackCount !== totalCallbacks) { | |
| hookHeader.html(`${originalText} <small>(${visibleCallbackCount}/${totalCallbacks} callbacks)</small>`); | |
| } | |
| } | |
| }); | |
| let hookHasMatchingCallbacks = !pluginFilter; | |
| let visibleCallbackCount = 0; | |
| if (pluginFilter) { | |
| // Filter individual callbacks within this hook group | |
| const callbackItems = $(this).find('.wp-hook-profiler-callback-item'); | |
| callbackItems.each(function() { | |
| const callbackMeta = $(this).find('.wp-hook-profiler-callback-meta'); | |
| const metaText = callbackMeta.text(); | |
| // Check if this callback matches the plugin filter | |
| const matchesPlugin = metaText.includes(`Plugin: ${pluginFilter}`) || | |
| metaText.toLowerCase().includes(`plugin: ${pluginFilter.toLowerCase()}`) || | |
| metaText.toLowerCase().includes(pluginFilter.toLowerCase()); | |
| if (matchesPlugin) { | |
| hookHasMatchingCallbacks = true; | |
| visibleCallbackCount++; | |
| $(this).show(); | |
| } else { | |
| $(this).hide(); | |
| } | |
| }); | |
| } else { | |
| // No plugin filter - show all callbacks | |
| $(this).find('.wp-hook-profiler-callback-item').show(); | |
| hookHasMatchingCallbacks = true; | |
| visibleCallbackCount = $(this).find('.wp-hook-profiler-callback-item').length; | |
| } | |
| // Show/hide the entire hook group based on whether it has matching callbacks | |
| const shouldShowHook = matchesSearch && hookHasMatchingCallbacks && visibleCallbackCount > 0; | |
| $(this).toggle(shouldShowHook); | |
| // Update hook header to reflect filtered callback count (preserve time span) | |
| if (shouldShowHook && pluginFilter) { | |
| const totalCallbacks = $(this).find('.wp-hook-profiler-callback-item').length; | |
| const hookHeader = $(this).find('.wp-hook-profiler-hook-header'); | |
| let countEl = hookHeader.find('.wp-hook-profiler-hook-count'); | |
| if (!countEl.length) { | |
| countEl = $('<small class="wp-hook-profiler-hook-count"></small>').appendTo(hookHeader); | |
| } | |
| if (visibleCallbackCount !== totalCallbacks) { | |
| countEl.text(` (${visibleCallbackCount}/${totalCallbacks} callbacks)`); | |
| } else { | |
| countEl.remove(); | |
| } | |
| } else { | |
| $(this).find('.wp-hook-profiler-hook-header .wp-hook-profiler-hook-count').remove(); | |
| } | |
| }); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes