fix: DISASM view is too slow#2168
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets performance bottlenecks in the Avalonia disassembly view by reducing per-scroll work (resource lookups, inline rebuilding, operand evaluation churn) and removing expensive smooth-scroll animation.
Changes:
- Moved disassembly brush resources to app-level resources and added brush caching for formatter kinds.
- Added caching for generated
InlineCollections and for compiled operand-evaluation expressions. - Simplified disassembly layout (removed shared-size groups) and switched scroll-to-address to instant scrolling.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Spice86/Views/Styles/DisassemblyResources.axaml | Updates breakpoint enabled colors for Light/Dark themes. |
| src/Spice86/Views/DisassemblyView.axaml.cs | Caches the ListBox reference to avoid repeated visual-tree lookups during scroll. |
| src/Spice86/Views/DisassemblyView.axaml | Removes per-view merged dictionary + shared sizing groups to reduce layout overhead. |
| src/Spice86/Views/Converters/FormatterTextKindToBrushConverter.cs | Adds theme-variant-aware brush lookup + caching, plus a cache version counter. |
| src/Spice86/Views/Converters/FormattedTextOffsetsConverter.cs | Adds InlineCollection caching and switches from dynamic-resource binding per Run to direct brush assignment. |
| src/Spice86/Views/Behaviors/DisassemblyScrollBehavior.cs | Removes smooth scrolling animation and uses direct offset changes. |
| src/Spice86/ViewModels/Services/ExpressionEvaluationService.cs | Adds caching of compiled expression delegates to reduce repeated JIT/compile overhead. |
| src/Spice86/ViewModels/DisassemblyViewModel.cs | Avoids re-evaluating operands for already-processed visible lines; skips redundant visible-range updates. |
| src/Spice86/App.axaml | Moves disassembly resource dictionary inclusion to application resources. |
Comments suppressed due to low confidence (1)
src/Spice86/Views/Converters/FormattedTextOffsetsConverter.cs:52
- Theme-change invalidation is currently lazy:
BrushCacheVersiononly increments whenGetBrushdetects a theme mismatch, butConvertcan return cachedInlineCollections without callingGetBrush. After switching Light/Dark, the disassembly may keep the old brushes. Consider making the version check explicitly observeApplication.Current.ActualThemeVariant(or subscribe to theme changes) so cached inlines are rebuilt deterministically on theme switch.
int currentVersion = FormatterTextKindToBrushConverter.BrushCacheVersion;
if (_cache.TryGetValue(textOffsets, out CachedEntry? cached) && cached.Version == currentVersion) {
return cached.Inlines;
}
Comment on lines
19
to
+35
| private readonly BreakpointConditionCompiler _compiler; | ||
|
|
||
| // Cache compiled Func<uint> by expression string. | ||
| // Expression.Compile() invokes the JIT and can take 2-10ms per call; caching makes | ||
| // repeated evaluations (across instructions and scroll events) effectively free. | ||
| private readonly Dictionary<string, Func<uint>> _compiledValueCache = new(); | ||
|
|
||
| public ExpressionEvaluationService(State state, IMemory memory) { | ||
| _compiler = new BreakpointConditionCompiler(state, memory); | ||
| } | ||
|
|
||
| private Func<uint> GetOrCompileValue(string expression) { | ||
| if (_compiledValueCache.TryGetValue(expression, out Func<uint>? cached)) { | ||
| return cached; | ||
| } | ||
| Func<uint> compiled = _compiler.CompileValue(expression); | ||
| _compiledValueCache[expression] = compiled; |
kevinferrare
force-pushed
the
bugfix/disasm_view_too_slow
branch
from
9d28b04 to
276c29e
Compare
maximilien-noal
approved these changes
May 13, 2026
Member
|
Looks good to me ! |
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
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.
Description of Changes
Disassembly view was taking 20sec to show something on my machine that is not a dinosaur.