Add opt-in startup and renderer performance tracing#15
Open
vogella wants to merge 1 commit into
Open
Conversation
![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 StartupTrace utility to measure and record the duration of various Eclipse workbench initialization phases. It instruments several key registries and services, such as Workbench, ActionSetRegistry, and ViewRegistry, to capture performance data that is dumped to a CSV file upon JVM shutdown. The review feedback suggests adding a null check for the user.home system property to prevent a potential NullPointerException and improving the CSV escaping logic to correctly handle carriage return characters.
| return; | ||
| } | ||
| try { | ||
| Path dir = Path.of(System.getProperty("user.home"), ".eclipse"); //$NON-NLS-1$ //$NON-NLS-2$ |
There was a problem hiding this comment.
The user.home system property is not guaranteed to be non-null in all environments. If it is null, Path.of(null, ".eclipse") will throw a NullPointerException. It is safer to check for null before proceeding.
Suggested change
| Path dir = Path.of(System.getProperty("user.home"), ".eclipse"); //$NON-NLS-1$ //$NON-NLS-2$ | |
| String userHome = System.getProperty("user.home"); //$NON-NLS-1$ | |
| if (userHome == null) { | |
| return; | |
| } | |
| Path dir = Path.of(userHome, ".eclipse"); //$NON-NLS-1$ |
| if (s == null) { | ||
| return ""; //$NON-NLS-1$ | ||
| } | ||
| if (s.indexOf(',') < 0 && s.indexOf('"') < 0 && s.indexOf('\n') < 0) { |
There was a problem hiding this comment.
The CSV escaping logic should also check for the carriage return character (\r). If a string contains \r but none of the other trigger characters, it will be returned unquoted, which can break the CSV structure in many parsers.
Suggested change
| if (s.indexOf(',') < 0 && s.indexOf('"') < 0 && s.indexOf('\n') < 0) { | |
| if (s.indexOf(',') < 0 && s.indexOf('"') < 0 && s.indexOf('\n') < 0 && s.indexOf('\r') < 0) { |
vogella
force-pushed
the
startup-trace-instrumentation
branch
3 times, most recently
from
80ad9db to
6eaa1a7
Compare
vogella
force-pushed
the
startup-trace-instrumentation
branch
from
9e65761 to
dfa878f
Compare
vogella
changed the title
Three layers of tracing emit CSV summaries on JVM shutdown, so we can measure where Workbench startup, renderer execution, and runtime UI events actually spend time. StartupTrace (opt-in via -Declipse.startup.trace=true) reuses the shared StartupTrace from org.eclipse.core.internal.runtime so workbench and platform bundles share one ~/.eclipse/startup-trace.csv and one runId. Instrumentation points cover Workbench.init phases, E4 application bootstrap, ResourceHandler, ModelAssembler, IDEApplication.start, and the first-touch of context functions in WorkbenchPlugin (PerspectiveRegistry, ViewRegistry, ActionSetRegistry, IntroRegistry, PreferenceManager, ThemeRegistry, WorkingSetManager, WorkingSetRegistry, EditorRegistry). With the property unset the cost is one static-final boolean read per call site. The same StartupTrace channel also captures runtime UI hotspots: - DisplayEventProbe installs SWT.PreEvent/SWT.PostEvent listeners on the workbench Display and records any event whose dispatch exceeds 16 ms, keyed by SWT event type (Paint, Selection, MouseDown, etc.). - WorkbenchLabelProvider.getText/getImage are bracketed so cumulative label-provider cost shows up per viewer refresh. - AbstractCSSEngine.applyStyles records the outermost call (depth counter avoids inflating the CSV with every recursive child). RendererPerfTracer is always active in this debug build and writes to ~/renderer-perf-trace.csv (configurable via -Declipse.renderer.perf.trace.file=<path>). It covers 14 hotspots across AreaRenderer, MenuManagerRenderer, MenuManagerRendererFilter, PerspectiveStackRenderer, SashRenderer, StackRenderer, ToolBarManagerRenderer, ToolControlRenderer, ToolItemUpdater, and WBWRenderer, with a persistent writer, shutdown hook, and daemon flusher. The H10 showTab probe is split into separate lazy-create and reparent paths. docs/performance.md documents both tracers, the captured trace sessions, and the analysis approach. Not intended for upstream merge; depends on vogella/eclipse.platform#4 for org.eclipse.core.internal.runtime.StartupTrace.
vogella
force-pushed
the
startup-trace-instrumentation
branch
from
dfa878f to
fb15507
Compare
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.
Two complementary tracers for measuring where Eclipse actually spends time, instead of estimating from code inspection. Both emit CSV summaries on JVM shutdown.
StartupTrace is opt-in via
-Declipse.startup.trace=trueand reuses the shared class fromorg.eclipse.core.internal.runtime(introduced by vogella/eclipse.platform#4), so workbench and platform bundles share one~/.eclipse/startup-trace.csvand onerunId. Instrumented points cover Workbench.init phases, E4 bootstrap (E4Application, ResourceHandler, ModelAssembler), IDEApplication.start, and the first-touch of context functions in WorkbenchPlugin (PerspectiveRegistry, ViewRegistry, ActionSetRegistry, IntroRegistry, PreferenceManager, ThemeRegistry, WorkingSetManager, WorkingSetRegistry, EditorRegistry). With the property unset, the cost is one static-final boolean read per call site.RendererPerfTracer is always active in this debug build and writes to
~/renderer-perf-trace.csv(configurable via-Declipse.renderer.perf.trace.file=<path>). It covers 14 hotspots across AreaRenderer, MenuManagerRenderer, MenuManagerRendererFilter, PerspectiveStackRenderer, SashRenderer, StackRenderer, ToolBarManagerRenderer, ToolControlRenderer, ToolItemUpdater, and WBWRenderer, with a persistent writer, shutdown hook, daemon flusher, and a deterministic top-N summary. The H10 showTab probe is split into separate lazy-create and reparent paths.docs/performance.mddocuments both tracers, captured trace sessions, and the analysis approach.Not intended for upstream merge; depends on vogella/eclipse.platform#4 for the shared
StartupTraceclass. Supersedes #13.