iOS: cut idle memory and startup cost in the Metal renderer and the VM - #5598
Open
shai-almog wants to merge 7 commits into
Open
iOS: cut idle memory and startup cost in the Metal renderer and the VM#5598shai-almog wants to merge 7 commits into
shai-almog wants to merge 7 commits into
Conversation
Six independent fixes, each measured with a paired A/B inside one binary (separate builds cannot resolve these: the same binary's footprint varies by tens of megabytes between launches depending on machine state, which is how several of these went unnoticed). Default-on: * Stencil attachments become Memoryless on tile-based GPUs. Every pass that binds the stencil is already loadAction=Clear / storeAction=DontCare, which is exactly the Memoryless contract; the code chose Private and reasoned the cost was "tiny (1 byte/pixel)". It is 5.6MB per allocation at 2048x1536. Worth ~3.8MB, 3/3 paired reps, Metal validation clean. Intel Macs and the Intel-Mac CI simulators still get Private via a runtime family probe. * The glyph atlas starts at 256x256 instead of 1024x1024. The atlas cache keys on postScriptName|SIZE, so every distinct text size reserved a megabyte of R8 before rasterising a single glyph, and a Material-style text theme has fifteen of them. tryGrowAtlas already doubles on demand and re-rasterises correctly, so starting small is self-correcting. 6.19MB -> 0.47MB, and no atlas in the test app ever needed to grow. * cn1SetupMetal no longer sizes the framebuffer from self.bounds before layout. An unlaid-out view reports the whole DISPLAY, so every launch allocated a 3456x2234 / 30MB screen texture, cleared it, blitted it into the real 2048x1536 one and threw it away. layoutSubviews always follows and sizes it correctly. Worth ~14MB, and it removes a +-20MB launch-to-launch swing that made every other memory measurement in this area unreliable. * Eager accessibility projection is gated on assistive technology actually running. The port sets isAccessibilityTreeSupported() = true and never overrode isAccessibilityTreeUpdateRequired(), which defaults to returning it, so the portable semantic tree was rebuilt on EVERY invalidation -- every layout, every scroll, every text setter -- on every device, whether or not VoiceOver was listening. The base class documents the opposite for pull-based ports, and UIKit pulls. 4.0MB of live allocation on an idle app with no assistive technology, ~6MB of footprint, plus the CPU to build it. Opt-in (CN1_DIRECT_DRAWABLE=1): * A direct-to-drawable render path with no retained screen texture: the frame's pass targets the drawable itself and present is a bare presentDrawable with no blit. Because the layer vends a different buffer each frame, partial repaint is incorrect there, so IOSImplementation.paintDirty enqueues the whole Form whenever anything is dirty -- reusing two existing behaviours rather than rewriting paintDirty. Worth ~8-11MB and ~11ms off first frame. Also sets maximumDrawableCount to 2 in that mode only, where the third slot really is taken (the cap is inert in the retained path). Diagnostics, compiled out unless defined: * CN1_ALLOC_CENSUS -- allocation volume by class, counted at all three allocation entry points so it does not silently miss the BiBOP and nursery objects a walk of allObjectsInHeap cannot see, plus cn1HeapAccounting, which separates Java-object storage from native allocation (vmmap cannot: BiBOP arenas and every Metal/CG buffer share the same malloc zones). * CN1_TEXTURE_CENSUS -- GPU memory by creation site with each large texture's true dimensions, and CN1_VERIFY_PRESENT, which reads back a patch of the drawable actually being presented. That last one matters: a renderer change can leave the screen blank while Metal validation stays clean and an offscreen repaint-based screenshot still looks perfect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1451368cd6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… probe General ParparVM memory/startup work that has been sitting unmerged, plus the fixes for the first review pass. * Lazy string constant pool. Every string constant in the binary was materialised into a java.lang.String before main ran -- on a large transpiled application, 38,238 of them. They are now created on first reference. * The force-visited table allocates its entries from arena blocks and prunes on sweep, instead of growing without bound and doing per-entry allocation on the hot path. * CN1_STARTUP_PHASES, a compiled-out probe that times the phases before the first frame, and the heap histogram alongside the allocation census. Review fixes: * cn1StencilStorageMode probes respondsToSelector: before sending supportsFamily:. That selector is iOS 13, ios.deployment_target is a build hint, and IPhoneBuilder will emit targets well below it -- where the message would have terminated the app during view initialisation. * updateFrameBufferSize: returns early in direct mode instead of building a render pass with a nil colour attachment and no explicit dimensions. That is invalid Metal; it survived only because the encoder came back nil and every message to it was a no-op. The polygon-clip stencil both paths need is factored into buildStencilTextureForWidth:height:layer:. * The assistive-technology check adds AssistiveTouch, and the VoiceOver / SwitchControl / AssistiveTouch status notifications now force a projection. Without that the native tree -- which is push-only, populated solely by accessibilityTreeChanged -> updateAccessibilityTree -- stayed empty when a technology started after a static screen was already up. UIKit publishes running flags for exactly those three and nothing else, so no flag can describe Voice Control or Full Keyboard Access. Rather than leave those users without a tree, any status notification latches eager projection on for the rest of the process: one process's worth of projection is the right side to err on for an accessibility feature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
Cloudflare Preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79971b3cce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
…gy flags The previous round gated eager projection on UIAccessibilityIsVoiceOverRunning / IsSwitchControlRunning / IsAssistiveTouchRunning and latched on their status notifications. Those are the only running flags UIKit publishes, so nothing in that set can see Voice Control or Full Keyboard Access -- and when one of them is already enabled at launch no notification fires either, so the latch never engaged and those users got no tree at all. METALView now overrides accessibilityElements. UIKit asks a container for its elements only when something is actually consuming the semantic tree, so the query itself is the signal that a client exists, and it depends on no per-technology flag. The first query may see a tree that has not been projected yet; noting the client schedules that projection and the resulting layout-changed notification brings the client back for the real one. Not covered: CodenameOne_GLViewController re-roots self.view to a plain UIView when a peer component is added mid-transition, and the elements are then set on that view instead. There the gate falls back to the flags and notifications -- the behaviour without this hook, not something worse. Noted at the override. Also documents why cn1_copyMetalScreenTextureImage returns NULL in direct-to-drawable mode: there is no retained screen texture to read, so Display.screenshot() falls through to drawViewHierarchyInRect:. That is correct on device but samples the presented drawable, so it can lag a frame and, on headless Catalyst with no display link, can be stale indefinitely. Reading the live drawable is not the fix -- retaining it past present starves nextDrawable, and after present the buffer is recycled. A deterministic capture wants a one-shot render into a scratch target, worth doing when the mode stops being opt-in. The default path is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d98105c56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ility APIs Two fixes from review. The shelf packer only ever tested growth against HEIGHT. That was safe while the atlas started at 1024x1024 and glyphs were capped at CN1_METAL_ATLAS_GLYPH_MAX (256): a fresh shelf always had room across, so the missing width test could never fire. Starting the atlas at 256 makes it reachable with ordinary glyphs -- a 256-wide glyph placed at x=1 in a 256-wide texture runs past the edge, and the region handed to replaceRegion: is out of bounds. Growth is now driven by a loop that tests both dimensions and terminates either by fitting or by hitting the growth ceiling, where the glyph is dropped exactly as an oversized one already was. UIAccessibilityAssistiveTouchStatusDidChangeNotification and UIAccessibilityIsAssistiveTouchRunning() are iOS 10, and ios.deployment_target lets IPhoneBuilder emit older targets. On those the weakly-linked constant is nil -- and a nil inside an @[] literal raises, so the observer registration would have taken the app down on the first accessibility invalidation -- while the running check would call through a null symbol. The notification list is now built up skipping absent constants, and the running check tests the function pointer before calling it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0669758bc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…r restart blurScreenRegionX, glassScreenRegionX and lensScreenRegionX each end the frame's encoder and reopen one on the SAME drawable via setFramebuffer. Direct mode set loadAction=Clear unconditionally, so every one of those effects wiped everything painted before it and presented only the effect and whatever followed. The restart site already carried the comment "loadAction Load preserves screenTexture" -- true of the retained path, and exactly the assumption direct mode broke. Only the first pass of a frame clears now: directFrameCleared is reset when a fresh drawable is vended (its contents are two or three presents old, so there is nothing to preserve) and set once the clear has happened, after which mid-frame restarts load. Why the earlier verification missed it: direct mode was checked by comparing the presented-frame hash against the retained path, which matched exactly -- on a screen with no glass or blur component in view. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2ddeaa43c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…fixes All four are in code this PR introduces. * The lazy constant pool was a data race. The writer stored under constantPoolMutex while every fast-path reader loaded plain, and those readers never take the mutex -- so it serialised writers and established nothing with them. A reader could observe the published pointer while the String's fields were still invisible, which on arm64 is not theoretical. Entries are now released by the writer and acquired by readers, the GC's mark scan included: the collector marks through that pointer and must see a constructed object behind it. * EAGLView gets the same accessibilityElements override METALView has. CodenameOne_GLViewController installs EAGLView whenever CN1_USE_METAL is absent, so on the GL backend the query latch was never installed and every portable-tree invalidation was discarded. Both call sites are gated on !TARGET_OS_WATCH, matching the note function, or the watch build fails to link. * paintDirty paints the full form directly instead of enqueueing it. repaint(f) appends and the superclass drains in order, so the full-frame paint landed on top of overlay animations already queued -- Container.TransitionAnimation queues its Transition through Display.repaint(t), and painting the form over it makes a component transition vanish or snap to its end state. The background has to go down first and the queue drain on top of it. * Class.getSuperclass() returns null for an interface. A class file records java/lang/Object as an interface's super_class and Parser.visit copies it into baseClass, so the isInterface flag is the only thing that separates them -- the function's own comment already claimed this behaviour while the code returned Object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30d4c327e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…le clear Follow-up to the previous commit, which fixed the transition ordering by painting the Form directly instead of enqueueing it -- and left super.paintDirty() still deriving its flush rectangle from the queued components alone. That rectangle is not advisory. CodenameOne_GLViewController hands it to ClipRect.setDrawRect and the Metal path clamps every screen op to it. So with a single partially dirty Component queued, direct mode cleared the ENTIRE drawable, repainted the whole Form, and then clipped that paint to the component's rect -- presenting a mostly black frame. Two changes each correct alone: clearing the whole drawable is right for a buffer that is two or three presents old, and a partial flush rect is right when only dirty regions are repainted. Direct mode makes them mutually exclusive -- clearing everything obliges you to flush everything -- and flushGraphics is where the two meet, so it widens to the full screen there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
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.
What this is
Six independent memory/startup fixes in the iOS Metal renderer and the VM, plus
the diagnostics that found them. Four are default-on, one is opt-in, and the
tooling is compiled out unless you define it.
Every number below is a paired A/B inside a single binary, toggled by an env
var. That matters more than it sounds: the same binary's physical footprint
varies by tens of megabytes between launches depending on machine state, so
"measure, change code, measure again" cannot resolve a 5MB effect and will
happily report noise as a win. Two of the fixes below were previously dismissed
on exactly that kind of measurement.
Default-on
Stencil attachments become Memoryless on tile-based GPUs. Every pass that
binds the stencil is already
loadAction=Clear/storeAction=DontCare, whichis precisely the Memoryless contract. The code chose
Privateand reasoned thecost was "tiny (1 byte/pixel)" — it is 5.6MB per allocation at 2048x1536.
~3.8MB, 3/3 paired reps, Metal validation clean. Intel Macs and the
Intel-Mac CI simulators keep
Privatevia a runtime family probe.The glyph atlas starts at 256x256 rather than 1024x1024. The atlas cache
keys on
postScriptName|SIZE, so every distinct text size reserved a megabyteof R8 before rasterising a single glyph — and a Material-style text theme has
fifteen of them across two or three families.
tryGrowAtlasalready doubles ondemand and correctly drops slots so the next reference re-rasterises, so
starting small is self-correcting. 6.19MB -> 0.47MB, and no atlas in the
test app ever needed to grow.
cn1SetupMetalno longer sizes the framebuffer before layout. It readself.bounds, and an unlaid-out view reports the whole display: every launchallocated a 3456x2234 / 30MB screen texture, cleared it, blitted it into the
real 2048x1536 one and threw it away.
layoutSubviewsalways follows and sizesit correctly, and
createRenderPassDescriptoralready treats a nilscreenTextureas "no frame this pass". ~14MB, and it removes a +-20MBlaunch-to-launch swing that was making every other measurement in this area
untrustworthy.
Eager accessibility projection is gated on assistive technology running.
The port sets
isAccessibilityTreeSupported() = trueand never overrodeisAccessibilityTreeUpdateRequired(), which defaults to returning it — so theportable semantic tree was rebuilt on every invalidation: every layout,
every scroll, every text setter, on every device, whether or not VoiceOver was
listening. The base class documents the opposite for pull-based ports, and UIKit
pulls. Measured at 4.0MB of live allocation on an idle app with no assistive
technology running at all, ~6MB of footprint, plus the CPU to build it.
Turning VoiceOver on mid-session is picked up on the next invalidation.
This last one affects every Codename One iOS application, not only the one it
was found on.
Opt-in:
CN1_DIRECT_DRAWABLE=1A direct-to-drawable path with no retained screen texture — the frame's pass
targets the drawable itself and present is a bare
presentDrawablewith noblit. ~8-11MB and ~11ms off first frame (13 paired reps, t = 2.74, lower in
11/13).
The layer vends a different buffer each frame, so partial repaint is incorrect
there — a region left unpainted shows a frame from two or three presents ago.
IOSImplementation.paintDirtytherefore enqueues the whole Form wheneveranything is dirty, reusing two behaviours that already exist rather than
rewriting
paintDirty: a component queued with a null dirty region alreadypaints under a full-screen clip, and
repaint(Animation)already drops a childwhose ancestor is queued, so it collapses the queue instead of growing it.
isDirectToDrawable()asks the renderer rather than deciding independently, sothe two halves cannot disagree about which buffer is being drawn into.
It also sets
maximumDrawableCountto 2 in that mode only, where the thirdslot really is taken (21.8MB vs a 21.8/27.2MB swing). The cap is inert in the
retained path — that was measured twice, and the comment now says so.
Diagnostics (compiled out unless defined)
CN1_ALLOC_CENSUS— allocation volume by class, counted at all threeallocation entry points, so unlike a walk of
allObjectsInHeapit does notsilently miss the BiBOP and nursery objects, which is exactly where small
high-churn objects live. Plus
cn1HeapAccounting, which separates Java-objectstorage from native allocation —
vmmapcannot, because BiBOP arenas and everyMetal/CoreGraphics buffer share the same malloc zones.
CN1_TEXTURE_CENSUS— GPU memory by creation site, logging each large texture'strue dimensions. Dimensions matter: a per-site cumulative total divided by its
allocation count invents a texture that does not exist, and that fabricated
figure sent me down a wrong path until the real sizes were printed.
CN1_VERIFY_PRESENT— reads back a patch of the drawable actually beingpresented. This is the only thing that can prove a renderer change did not
blank the screen: with a nil drawable the render pass is nil, every op no-ops
against a null encoder, and Metal validation stays clean while
FIRSTFRAMEstill prints. An offscreen repaint-based screenshot cannot catch it either,
because it repaints the scene graph instead of reading the framebuffer.
Verification
hellocodenameonebuilds as a Mac native target with zero errors, bothwith default settings and with all three diagnostics defined (the latter is
necessary — a default build compiles straight past the guarded code).
🤖 Generated with Claude Code