Complete Performance Overhaul - #1036
Merged
Merged
Conversation
… JVM names Kotlin mangles every internal member's JVM name with the module name, and 2.4 changed that default from the archives name to the maven coordinates. That renames a published ABI on a toolchain bump: PolyPlus, built against an earlier release, stopped linking against ThemeRegistry.getRegistry$internal the moment it moved, which left no UI and a blank main menu.
Packet and entity render events fire thousands of times a second and allocated an event object every time even with no listeners; hasListeners lets the hot call sites skip it. The handler map is now concurrent, since packet events post from the netty loop while mods register from the main thread, so a handler could stay invisible indefinitely. The pools were built behind unsynchronised null checks, so two callers racing each built one and leaked one, and the scheduled pool asked for a negative thread count on any machine with fewer than three cores.
…hrough overwrite() reports "cannot write through this" by throwing, and set0 runs once a frame for the length of a slider drag, so every one of those filled in a stack trace. Ask isImmutable first, on the value's runtime class, which is the same question overwrite asks. The reflective collector stopped walking into Object, which declares no config properties and cost eleven reflected methods per config per nesting level, and its two null guards ran in the wrong order so the error path threw instead of explaining itself.
A composition only does work on a frame, so splitting the single host into clocks lets the HUD previews be kept alive between opens and simply left alone the rest of the time, instead of being rebuilt on every open. frame() also takes notify, because sendApplyNotifications is global and firing it twice in one frame hands new state to a tree already composed against the old.
prepare() filtered a fresh list of active HUDs on every frame; it now reuses one. The update schedule moved off a HashMap keyed by Hud onto a field on the HUD itself, which is a lookup and a hash per HUD per frame saved. hasHudOfType answers what the library asks once per provider per recomposition without building a list to do it.
Every open reloads the registry, which registered every tree again, which walked all of them: the whole property graph plus a localised title each, measured at 45 ms on the frame the interface opened. The registrar now remembers the trees it has walked, weakly so a discarded profile is not kept alive, and a caller that means it can still force a rescan. loadFrom also stopped scanning each tree twice. configList copied the whole registry on every read, and category, version and description each scanned the entire mod list per tree. A star click no longer puts a disk write in the middle of the frame it happened in.
… has Compose's bitmap painter built and destroyed a native image on every draw, and the mod list draws over a hundred icons a frame: the finalisers those queued held the render thread for 320 ms. Nothing about the image changes between draws, so one raster is shared by every call site and tinted at draw time. An icon was also re-parsed every time a card scrolled back into view, because the read lived in a remember the lazy grid throws away. Caching the raster took the mean scroll frame from 17.2 ms to 5.1 ms and the worst second from 64.7 to 19.3 ms. A classpath miss walks every jar the game loaded, 139 here including a 78 MB compose bundle, and nearly every icon probe misses by design, so misses are cached too and names are resolved off the render thread beforehand.
…rame A HUD reads live values, and reading them inside a draw scope makes the layer depend on them, so one ticking clock invalidated the layer and one invalidated layer redraws the whole interface. Reading them outside observation still draws current values on any frame that draws, and dragging, selecting and resizing all still drive frames of their own. The previews are also kept in a cache of their own now, since a lazy grid disposes cards that scroll out of view and rebuilding the runtime and re-running setContent is the most expensive thing a card does.
Waiting on the frame clock unconditionally leaves an awaiter on it forever, and Compose reads a pending awaiter as pending work, so every frame came back dirty and redrew the whole screen. Only a chroma accent changes on its own, and the accent colour is a plain static rather than snapshot state, so that flag needs to be state of its own for the effect to notice it change.
…building brushes The grid rebuilt a card's whole subcomposition and node tree for each card scrolling in. A card costs about 2 ms to arrive and only 0.26 ms of that is composing it, so a contentType lets the grid reuse one scrolling out instead. Hovering recomposed the entire card, both gradients and both labels and the icon, for a change only the favourite star cares about; the star takes the interaction source now rather than its state. Scroll edge gradients were rebuilt every frame on every scrollable surface, and a card drew its two decorations in two nodes when one will do. The shell's glow opacity was read inside the draw, which makes the layer depend on it, and the shell background is the one thing drawn on every frame of every page. The search results grid had stopped being lazy: the whole grid sat in a single lazy item, so a broad query composed most of the mod list on the keystroke that produced it. As rows it stays lazy, and sizing in pixels stops the last column wrapping.
AnimatedVisibility disposes its content when the exit animation ends, which for a retained scene hands back the whole cost of an open. RetainedVisibility animates the same way and simply stops drawing. Its modifier chain is deliberately the same shape shown or hidden: a modifier that comes and goes adds and removes nodes Compose indexes for hit testing, and toggling one desynchronised that index and took the scene down. The composition holding the nav controller now outlives a close, so only the very first one is already sitting on the start destination and every open after it has to navigate. Grid states are published so a warm-up can scroll a list before anyone looks at it.
Composing, laying out and drawing the tree is nearly all of what an open costs, and all three are skipped when the composition is still there. Reopening went from about 72 ms to about 9 ms. The HUD editor is retained too, since it is the most expensive open OneConfig has: forty preview runtimes and the whole studio. Three things a retained composition needs. The interface raises its visibility from a LaunchedEffect(Unit) that runs once per composition, so a screen coming back after a close has to ask for it as cancelClose does. A screen still carries the close it was dismissed by, which left the opening frame computing a finished closing animation and rendering fully faded out. And the shell's last bounds would still answer hit tests once it is no longer laid out, which the HUD editor asks to decide what a click is over. A screen aimed at one mod's config is not shared: its page is fixed at construction and there is nothing to come back to. Holding the open key produced dozens of closes a second and therefore dozens of whole-config writes queued behind each other; saving is coalesced and serialised.
…ally open at The first frame that can warm up is not the frame worth warming: Minecraft's window is still 854x480 and most mods have not registered their configs, so one pass laid the whole UI out at the wrong size against a fraction of the data and the first real open paid 197 ms to do it again. It watches instead, and a pass that finds nothing changed costs three reads. Running the whole pass at once put it on a single frame, measured at 1332 ms as the world appeared, so it runs a frame at a time. Every page is warmed rather than only the mod grid, which took the worst section frame from 195 ms to 48 ms, and the grid is dragged through its whole extent because an icon is only rasterised the first time its card is actually drawn. Focusing the search field lays out a paragraph to report its rectangle, which drags in the entire Skia text stack: 40 ms on the frame the interface opened. The class warm list was 51 names typed by hand, five of them stale, so it is read off our own jars now. An idle frame no longer copies the draw list to find out it was empty.
The mixin reads hasContent earlier in the frame than the capture runs, so clearing it on the way in meant a frame that failed to record had already cancelled vanilla and then blitted nothing. Keeping the last capture makes the worst case a slightly stale frame rather than an empty one. Every path that fails to record now reports false so the caller falls back to its live renderer, since returning true without setting hasContent suppresses that renderer and leaves the HUD missing.
Retaining a composition and warming it offscreen turns every LaunchedEffect(Unit) in it from "once per open" into "once, ever", and drawing without observing reads leaves nothing to invalidate a layer. Three things broke on that. A HUD preview mirrors a provider rather than an active instance, so nothing advanced what it showed and nothing told its canvas to redraw. Providers are updated on their own interval while the UI is open, and a preview frame that changed something bumps a revision the canvas reads in its draw. One dependency rather than every live value the HUD touches, so a ticking clock costs one redraw a second instead of one a frame. The editor took its pending selection from a LaunchedEffect(Unit), which had already run against nothing during the warm-up, so clicking a HUD's settings opened a bare editor. An open bumps a revision those effects key on, which covers queued session commands too.
The pass was one 2430 ms frame on the title screen, which is a visible freeze on every game open. It waits for a loading screen now: singleplayer stops on LevelLoadingScreen, a server on ConnectScreen, and a world already loaded is where it used to run anyway. A session that never reaches a world gives up after ten minutes and lets the first open build the UI, as it did before any of this. setContent builds the whole composition and was rendering on the same frame, so it gets one of its own. The offscreen target is viewport sized and was allocated and destroyed on every frame of the pass, thirty times at 1920x1080; it is kept for the length of a pass instead. Worst frame 2430 ms to 837 ms, and no longer on a screen anyone is watching.
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.
28 performance fixes (click to expand, ranked worst first)
upsertcopied 138 entries and scanned them linearly, once per tree, on every open, and the HUD card lookups did the same once per card.Object, reflecting over its eleven methods for every config at every nesting level.12 bugs fixed (click to expand, ranked worst first)
RectListcrash.What this is
UI performance work on the Compose interface, plus the bugs found while doing it.
Why
On a 141 mod pack the interface was costing between a third and a half of every
frame while it was on screen, and single frames were reaching 190 to 327 ms.
Everything below was measured before and after by a scripted controlled run doing the same
thing every time (three opens, a tour of every page, a slow scroll and two flicks,
three HUD editor opens) on three machines. The harness itself is not part of this
PR.
Worst single frame, ms, before to after
What OneConfig cost per frame, ms
A 60 fps frame has 16.7 ms in total.
Frames past 33 ms that we caused: 32 → 5, 45 → 7, 19 → 3.
Frames past 50 ms: 28 → 2, 28 → 1, 4 → 0.
The larger pieces
The screen and its scene are reused. Composing, laying out and drawing the tree
is nearly all of what an open costs. Reopening went from ~72 ms to ~9 ms. This is
the change with the most surface area: a retained composition needs the interface
asked to raise its visibility again, the close state reset, and the shell's stale
bounds cleared so they stop answering hit tests.
The UI is built on the title screen. The old warm-up ran on the first frame it
could, when the window is still 854x480 and most mods have not registered, so the
first real open paid 197 ms to redo it. It now watches until the inputs settle and
warms a frame at a time rather than landing as one 1332 ms frame.
Icons are rasterised once. Compose's bitmap painter built and destroyed a native
image per draw and the mod list draws over a hundred a frame; the finalisers held
the render thread for 320 ms.
HUD previews stopped redrawing the whole interface. A HUD reads live values and
those reads happened inside a draw scope, which makes the layer depend on them, so
a ticking clock invalidated everything.
Bugs fixed
that link against them (PolyPlus: no UI, blank main menu). The module name is
pinned to the archives name.
composed against the old.
test index and took the scene down.
animations backwards.
scheduled pool asked for a negative thread count under three cores.
netty loop.
Two behaviour changes
constructed per open.
OneConfigUIScreen.open()replacesOneConfigUIScreen()at the call sites that want the shared one; a per-mod screen is still constructed
and is deliberately not retained.
and rebuilt on resize, so releasing them only made the next open pay for a fresh
texture, framebuffer and Skia surface.
Known problems with the mod, not fixed
where other pages sit at 0 or 1.
through Metal and a Skia flush there can block on the game's queued work, this is in every case and run to run variance is significant plus I dont own a damn APPLE product. That path is untouched by this PR.