Open
Conversation
Add RendererPerfTracer utility for lightweight CSV-based performance
tracing of 14 identified hotspots in SWT renderers. Tracing is always
active in this debug build.
Output goes to ~/renderer-perf-trace.csv (configurable via
-Declipse.renderer.perf.trace.file=<path>).
Usage:
1. Build the IDE with this commit
2. Launch the IDE normally
3. Use the IDE for a while (open/close editors, switch perspectives,
use menus, resize sashes, switch tabs, save files)
4. Close the IDE
5. Analyze ~/renderer-perf-trace.csv
CSV format: timestamp_ms,hotspot_id,duration_ns,detail
Instrumented hotspots (matching docs/performance.md):
- H01: findElements for CSS_ACTIVE_CLASS on every activation
- H02: findElements for placeholders on label/item changes
- H03: unbatched updateWidget/requestLayout in ToolBarManagerRenderer
- H04: ALL_SELECTOR update on every dirty flag change
- H05: ToolItemUpdater linear scan with ArrayList
- H06: unbatched scheduleManagerUpdate in MenuManagerRenderer
- H07: per-item EclipseContext creation in menu show
- H09: findElements for MPartStack on window activate/deactivate
- H10: showTab without setRedraw batching
- H11: limbo reparenting without setRedraw batching
- H12: synchCTFState recursive tree walk from getUIContainer
- H13: full model scan at app startup in ToolControlRenderer
- H14: uncoalesced RunAndTrack callbacks
- W1: synchronous layout(true,true) on Windows (SWT bug 558392)
- Add Session 2 measurements alongside baseline for comparison - H10 setRedraw fix landed (pending validation), H07 now top unfixed priority - H07 total time rose 68% (568ms to 954ms), call count nearly doubled - H03 showed new 6ms max spike worth monitoring - H06 frequency increased 83% (268/min to 490/min) - Add high-frequency hotspot table and fix status section
- Hold one BufferedWriter for the tracer's lifetime instead of reopening the file on every flush. - Drive flushes from a single daemon ScheduledExecutorService instead of Display.timerExec, removing the implicit UI-thread dependency so off-thread callers (e.g. RunAndTrack) are safe. - Register a JVM shutdown hook that drains the queue and closes the writer so the final 500 ms of events are not lost. - Rename the internal wallMs variable to elapsedMs; it is monotonic elapsed time since process start, not wall clock. - Add a comment explaining why H08 is skipped in the hotspot list.
Aggregate per-hotspot stats (count, total ns, max ns, p95 ns, calls/min) as events are recorded, and write a human-readable summary file next to the CSV on JVM shutdown. Path is <csv>.summary.txt, so the default is ~/renderer-perf-trace.csv.summary.txt. This removes the need to summarize the CSV with an external tool, which had produced at least one documented data artifact in session 2.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a lightweight performance tracer, RendererPerfTracer, and integrates it into various SWT renderers to monitor hotspots. It also performs a large-scale refactoring of logging calls to use the modern ILog API across multiple bundles. Significant improvements were made to FilteredItemsSelectionDialog to serialize background jobs and prevent race conditions, and AnimationManager was updated to use Display.timerExec for more efficient debouncing. Test stability was also addressed by preventing premature garbage collection of addons and utilizing the new job family for synchronization. Feedback is provided regarding the configurability of the performance tracer and a potential race condition in its asynchronous flush logic.
| public final class RendererPerfTracer { | ||
|
|
||
| /** Master switch — always enabled in this debug build. */ | ||
| public static final boolean ENABLED = true; |
There was a problem hiding this comment.
The performance tracer is currently hardcoded to be always enabled. For production use, this should be configurable (e.g., via a system property or a debug option) and disabled by default to avoid unnecessary overhead and file I/O for all users.
Suggested change
| public static final boolean ENABLED = true; | |
| public static final boolean ENABLED = Boolean.getBoolean("eclipse.renderer.perf.trace.enabled"); |
Comment on lines
+130
to
+141
| private static void flush() { | ||
| FLUSH_SCHEDULED.set(false); | ||
| try (BufferedWriter writer = Files.newBufferedWriter(OUTPUT_FILE, | ||
| StandardOpenOption.CREATE, StandardOpenOption.APPEND)) { | ||
| String line; | ||
| while ((line = QUEUE.poll()) != null) { | ||
| writer.write(line); | ||
| } | ||
| } catch (IOException e) { | ||
| // Silently drop — tracing must not break the workbench | ||
| } | ||
| } |
There was a problem hiding this comment.
Setting FLUSH_SCHEDULED to false at the beginning of the flush() method creates a race condition. If Display.getDefault() is null (e.g., in headless mode or during early startup), multiple threads calling trace() could enter flush() concurrently, leading to multiple threads attempting to write to the same file simultaneously. It is safer to reset the flag in a finally block and check if more items were added during the flush.
private static void flush() {
try (BufferedWriter writer = Files.newBufferedWriter(OUTPUT_FILE,
StandardOpenOption.CREATE, StandardOpenOption.APPEND)) {
String line;
while ((line = QUEUE.poll()) != null) {
writer.write(line);
}
} catch (IOException e) {
// Silently drop — tracing must not break the workbench
} finally {
FLUSH_SCHEDULED.set(false);
if (!QUEUE.isEmpty()) {
scheduleFlush();
}
}
}- Add a Methodology section (environment, workload, cold start vs steady state) so future sessions are reproducible. - Reference hotspots by method name instead of line numbers; line numbers rot. - Remove the speculative H07 root-cause recommendation: the two slowest recorded H07 calls have ctxCreated=0, so per-item context allocation is not the dominant cost on the tail path. Mark root cause as needing attribution. - Reword the H10 status: the candidate fix lives on a separate branch and has not been measured here. Drop the 'pending validation' phrase. - Move the menu-batching workaround (W2 / bug 467000) out of the platform-specific section into a cross-platform batching-gaps section, since it is not platform-specific. - Add H04 as a row in the ranking table (count-only; absent from the earlier sessions' summaries) so it appears in the next session. - Flag the H09 inconsistency as likely a summarization artifact and note that the next session should use the tracer's built-in summary output instead of an external summary step. - Strip em/en dashes throughout.
The setRedraw batching candidate (wip-fix-showTab-setRedraw-batching) was tried in an aggregator build and measured on Linux/GTK. It did not reduce H10 cost; it reattributed paint work into the method and inflated the tail. Measurement showed the tail in every session is one or two very expensive calls on the lazy-create path where renderer.createGui(element) runs for the first time. The reparent path stays in the single-digit ms range regardless of batching. The two paths have different causes and different ranges, so reporting them as one hotspot hides the signal. Split H10_showTab_noBatch into H10a_showTab_lazyCreate (first-show, widget was null) and H10b_showTab_reparent (widget existed, being reparented or reselected). Future sessions can now see per-path P95 and max. Performance doc updated with the measurement table from the abandoned setRedraw fix and the rationale for the split.
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
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.
No description provided.