[codex] Improve render-path responsiveness#388
Merged
schuyler merged 4 commits intoApr 20, 2026
Merged
Conversation
- Fix retain cycle in parseAndRenderWithMaxDelay: by replacing __block NSBlockOperation *operation with __weak reference pattern. The old pattern created operation→block→operation cycle that leaked the renderer and associated WebViews. - Move cleanupObsoleteAutosaveValues back to synchronous execution. Running it async on a background thread raced with app initialization and triggered a crash on macOS 26 when NSUserDefaults mutations interacted with WKWebView setup during test host startup. Related to schuyler#388
schuyler
requested changes
Apr 20, 2026
schuyler
left a comment
schuyler
left a comment
Owner
There was a problem hiding this comment.
Thanks for the contribution, @yusufm.
Issues:
openOrCreateFileForUrl: — the unsaved-document guard is missing a return, so users hit a second, misleading "Blocked file creation" alert from canAutomaticallyCreateLinkedFileAtURL: returning NO on nil fileURL.
The loop comment in updateHeaderLocations still says "splitting our document into lines", missing that the switch to NSStringEnumerationByLines now correctly handles \r\n and bare \r.
Minor suggestions: use a __strong local for the cancellation checks, add brief comments on the shared regex and 10ms poll interval, explain the cleanupObsoleteAutosaveValues reorder, add CHANGELOG.md entries for the three user-visible changes, and confirm the NSString (WordCount) header was removed.
Generated by Claude Code
yusufm
added a commit
to yusufm/macdown3000
that referenced
this pull request
Apr 20, 2026
- Update updateHeaderLocations comment to reflect NSStringEnumerationByLines (correctly handles \r\n and bare \r, not just splitting by \n). - Use __strong local for weakOp cancellation checks in the renderer polling loop to avoid weak reference going nil between checks. - Add brief comments on the shared regex dispatch_once and 10ms poll interval constant. - Add comment explaining cleanupObsoleteAutosaveValues reorder (runs after migration and defaults so cleanup does not remove just-set keys). - Add CHANGELOG.md entries for the three user-visible changes. - Confirmed: NSString (WordCount) category had no separate header; it was only in DOMNode+Text.m and has been fully replaced. Related to schuyler#388
schuyler
pushed a commit
that referenced
this pull request
Apr 20, 2026
Changelog updates for PRs are handled by the maintainer at release time. Related to #388
yusufm
force-pushed
the
Codex/perf-render-latency-019da736-8573-7ac1-9883-259ce39e964a
branch
from
bf4a41e to
ba747f6
Compare
yusufm
added a commit
to yusufm/macdown3000
that referenced
this pull request
Apr 20, 2026
- Update updateHeaderLocations comment to reflect NSStringEnumerationByLines (correctly handles \r\n and bare \r, not just splitting by \n). - Add brief comment on the cached body-extraction regex dispatch_once and the 10ms poll interval constant. - Add comment explaining cleanupObsoleteAutosaveValues reorder (runs after migration and defaults so cleanup does not remove just-set keys). - Add CHANGELOG.md entries for the three user-visible changes. - Confirmed: NSString (WordCount) category had no separate header; it was only in DOMNode+Text.m and has been fully replaced. - Note: __strong local for weakOp skipped. NSOperationQueue retains operations during execution, so weakOp cannot go nil mid-block. Related to schuyler#388
yusufm
added a commit
to yusufm/macdown3000
that referenced
this pull request
Apr 20, 2026
- Update updateHeaderLocations comment to reflect NSStringEnumerationByLines (correctly handles \r\n and bare \r, not just splitting by \n). - Add brief comment on the cached body-extraction regex dispatch_once and the 10ms poll interval constant. - Add comment explaining cleanupObsoleteAutosaveValues reorder (runs after migration and defaults so cleanup does not remove just-set keys). - Add CHANGELOG.md entries for the three user-visible changes. - Confirmed: NSString (WordCount) category had no separate header; it was only in DOMNode+Text.m and has been fully replaced. - Note: __strong local for weakOp skipped. NSOperationQueue retains operations during execution, so weakOp cannot go nil mid-block. Related to schuyler#388
yusufm
force-pushed
the
Codex/perf-render-latency-019da736-8573-7ac1-9883-259ce39e964a
branch
from
ba747f6 to
af0c688
Compare
yusufm
added a commit
to yusufm/macdown3000
that referenced
this pull request
Apr 20, 2026
- Update updateHeaderLocations comment to reflect NSStringEnumerationByLines (correctly handles \r\n and bare \r, not just splitting by \n). - Add brief comment on the cached body-extraction regex dispatch_once and the 10ms poll interval constant. - Add comment explaining cleanupObsoleteAutosaveValues reorder (runs after migration and defaults so cleanup does not remove just-set keys). - Add CHANGELOG.md entries for the three user-visible changes. - Confirmed: NSString (WordCount) category had no separate header; it was only in DOMNode+Text.m and has been fully replaced. - Note: __strong local for weakOp skipped. NSOperationQueue retains operations during execution, so weakOp cannot go nil mid-block. Related to schuyler#388
yusufm
force-pushed
the
Codex/perf-render-latency-019da736-8573-7ac1-9883-259ce39e964a
branch
from
af0c688 to
4b6134f
Compare
yusufm
added a commit
to yusufm/macdown3000
that referenced
this pull request
Apr 20, 2026
- Update updateHeaderLocations comment to reflect NSStringEnumerationByLines (correctly handles \r\n and bare \r, not just splitting by \n). - Add brief comment on the cached body-extraction regex dispatch_once and the 10ms poll interval constant. - Add comment explaining cleanupObsoleteAutosaveValues reorder (runs after migration and defaults so cleanup does not remove just-set keys). - Add CHANGELOG.md entries for the three user-visible changes. - Confirmed: NSString (WordCount) category had no separate header; it was only in DOMNode+Text.m and has been fully replaced. - Note: __strong local for weakOp skipped. NSOperationQueue retains operations during execution, so weakOp cannot go nil mid-block. Related to schuyler#388
yusufm
force-pushed
the
Codex/perf-render-latency-019da736-8573-7ac1-9883-259ce39e964a
branch
from
4b6134f to
6a0bb5c
Compare
- Update updateHeaderLocations comment to reflect NSStringEnumerationByLines (correctly handles \r\n and bare \r, not just splitting by \n). - Add brief comment on the cached body-extraction regex dispatch_once and the 10ms poll interval constant. - Add comment explaining cleanupObsoleteAutosaveValues reorder (runs after migration and defaults so cleanup does not remove just-set keys). - Add CHANGELOG.md entries for the three user-visible changes. - Confirmed: NSString (WordCount) category had no separate header; it was only in DOMNode+Text.m and has been fully replaced. - Note: __strong local for weakOp skipped. NSOperationQueue retains operations during execution, so weakOp cannot go nil mid-block. Related to schuyler#388
yusufm
force-pushed
the
Codex/perf-render-latency-019da736-8573-7ac1-9883-259ce39e964a
branch
from
6a0bb5c to
d512190
Compare
Owner
|
Looks good! Thank you @yusufm |
schuyler
added
the
rc-pending
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
2 participants
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.
Summary
Why
The previous render scheduler could hot-loop on
rendererLoading, which wasted CPU and stretched visible preview latency under load. The preview path also did extra whole-document work during live updates, and preferences cleanup was still blocking startup despite being non-urgent.Validation
xcodebuild test -workspace "MacDown 3000.xcworkspace" -scheme MacDown -destination "platform=macOS" -only-testing:MacDownTests/MPRendererStateTests -only-testing:MacDownTests/MPWordCountUpdateTestsxcodebuild test -workspace "MacDown 3000.xcworkspace" -scheme MacDown -destination "platform=macOS" -only-testing:MacDownTests/MPMarkdownRenderingTests/testVeryLargeDocument -only-testing:MacDownTests/MPScrollSyncTestsTiming Notes
Manual scheduler benchmark for the old vs new wait loop:
maxDelay=0.05s,loadingDuration=1.00s: old loop22,374,992polls /1.0000s, new loop6polls /0.0506smaxDelay=0.50s,loadingDuration=0.50s: old loop11,163,371polls /0.5000s, new loop44polls /0.5023s