[6.x] Improve Bard/Replicator performance on large documents#15057
Open
SteJW wants to merge 2 commits into
Open
[6.x] Improve Bard/Replicator performance on large documents#15057SteJW wants to merge 2 commits into
SteJW wants to merge 2 commits into
Conversation
Member
|
Hey @SteJW, Could you please elaborate on the following?
|
Contributor
Author
|
It turns out there was still a bug while adding new items to the page builder. I reverted the changes to work on it some more. |
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.
Summary
Performance fixes for the CP with large Bard/Replicator page-builder content
(see #13385). Builds on the unmerged parts of #14512 — combining its ideas
with an additional Vue-reactivity fix that PR was missing — plus one
unrelated bug found while investigating: the Publish button can get stuck
disabled after Save on large entries.
Scoped deliberately narrow, per @jasonvarga's note on #14512 ("way too much
to be in one PR"). This covers Bard-specific performance only. It does not
include #14512's Replicator/Grid-level changes (
replicator/Set.vue,Replicator.vue) — those are independent and better suited to their own PR.1.
BardFieldtype.vue—onUpdatecost on every transactiononUpdatepreviously did, on every Tiptap transaction (including plaincursor moves before this change):
For a large document this clones and recursively walks the whole tree twice
per keystroke. Separately,
this.jsonis a Vuedata()property, so Vuedeep-reactive-proxies the entire tree — every nested node gets its own Proxy
plus a dependency-tracking map, regenerated on every update. Traced with
Chrome DevTools: a single
RunMicrotaskstask blocked the main thread for~7s on one large real-world document, dominated by Vue's internal
track/triggermachinery walking that Proxy tree. Independentlycorroborated on #13385 by @jasonbaciulis, who profiled 8,295 microtasks and
~3.7s on a single Enter keypress with matching root cause.
This PR combines two complementary fixes:
change the document (
if (!transaction.docChanged) return;), and usedoc.content.sizeinstead of the recursive node count to detectstructural changes.
jsonassignments (both inmounted()andonUpdate) inmarkRaw(), so Vue never proxies the tree in the firstplace. This is the part [6.x] Improve CP performance: staggered set mounting, fallback previews, targeted condition watching #14512 didn't address — it removes the clone/walk
cost but leaves the full document reactive, so the Proxy/dependency-map
memory overhead (and the
jsonwatcher'sJSON.stringifycost elsewherein this file) remains.
markRaw()closes that gap without touchingmutability, so it's safe even though this data later gets read through
the entry's outer reactive
valuesstore.One behavior change worth discussing directly, not silently deciding:
doc.content.sizechanges on every keystroke, not just structuraledits — broader than the original
countNodes()'s intent (distinguishing"typed a character" from "added/removed a block"). If
debounceNextUpdatebeing reset more often than before turns out to matter, the safer fallback
is comparing
this.json.length(top-level node count) instead — cheaperthan the original recursive walk while preserving the original's coarser
granularity. Flagging this rather than picking one silently.
2.
Fieldtype.vue— targeted condition watching (from #14512, unmerged)publishContainerunwrapped every injected ref viaObject.fromEntries(Object.entries(...).map(...))— reading every ref's.valueup front. Since this is a Vuecomputed, it has a reactivedependency on all of them, so it recomputes for every field on the page
whenever any one of them changes, cascading condition re-evaluation
broadly instead of scoping to what each field's own condition actually
reads.
Replaced with a cache built once, using lazy per-key getters
(
Object.definePropertywithget: () => val.valuefor ref-backed keys).The outer object itself never invalidates; reading a specific key still
goes through to the live ref, so dependency tracking happens correctly in
whichever consumer's own reactive scope reads it.
Adjusted from #14512's original patch to preserve this branch's
UPDATE_DEBOUNCE_MSconstant (extracted since #14512 was forked) ratherthan reverting to a hardcoded
150.3.
Set.vue(Bard) — staggered field mounting (from #14512, unmerged, reconciled)New
resources/js/util/createMountScheduler.js(included as-is): batches"mount" callbacks and drains them incrementally via
requestIdleCallback(falling back to
requestAnimationFrame), yielding to the browser betweeneach and awaiting
nextTick()so Vue can flush DOM updates incrementallyinstead of all at once. This directly targets the root cause behind the
initial-render freeze: a document with many Sets currently mounts every
Set's full field UI synchronously in one block.
BardFieldtype.vuenow provides amountSchedulerinstance; eachSet.vuedefers rendering its own fields (
fieldsReady) until the scheduler gets toit, rather than all Sets doing so in the same tick on initial expand.
Reconciliation note: #14512's fork was based on an older snapshot of
the
6.xbranch and had drifted from current6.x— its version ofSet.vuewas missing RTL direction support (:dir="uiDirection") and adrag-selection fix (
preventNodeSelectionDrag) that have since landedupstream. This PR's
Set.vueis hand-reconciled against the currentbranch tip with both of those preserved; it is not a copy of #14512's file.
Also folded in from the same diff: a small
MutationObserverdebounce viarequestAnimationFrame(avoids redundantsetAttributecalls firingsynchronously on every mutation), and a
v-if+v-forcleanup on the sameelement in the field-actions dropdown (Vue anti-pattern, was also missing
a
:key).4.
PublishForm.vue— Publish button stuck disabled after SaveUnrelated to #14512/#13385's thread but found while investigating the same
class of large-document performance problem.
canPublishreturnsfalsewhenever
isDirtyis true. After Save,trackDirtyStateis setfalse,valuesis reset from the response, and after a fixed 500ms timeout,dirty-tracking re-enables (comment in source: "after any fieldtypes do a
debounced update"). On a large Bard document, the fieldtype's own
post-reset update cycle can exceed 500ms even after fix #1 above, so the
entry gets marked dirty again right after the grace window closes, and the
Publish button stays disabled until another edit or a full reload.
Bumped the fixed delay from 500ms to 3000ms across all 5 call sites in this
file (save, quick-save, editLocalization, createLocalization branches).
This is a mitigation, not a structural fix — a large enough document could
still exceed a fixed timeout. A more robust approach would track pending
fieldtype updates directly and wait for those to settle rather than
guessing a duration; happy to explore that instead if maintainers prefer.
Testing
RunMicrotasksblock ononUpdatedisappears after fix First #1.reactive()/markRaw()that afrozen-object approach (tried and rejected during development) throws
under nested reactive access, while
markRaw()does not — see priordiscussion, not repeated here.
createMountScheduler.jsincludes its original unit tests from [6.x] Improve CP performance: staggered set mounting, fallback previews, targeted condition watching #14512(
resources/js/tests/createMountScheduler.test.js), included unchanged.Save → Publish flow on a large entry.
Files changed
resources/js/components/fieldtypes/bard/BardFieldtype.vueresources/js/components/fieldtypes/Fieldtype.vueresources/js/components/fieldtypes/bard/Set.vueresources/js/util/createMountScheduler.js(new)resources/js/tests/createMountScheduler.test.js(new)resources/js/components/entries/PublishForm.vueCredits
Fixes #1 and #3 build directly on work originally proposed by @waldemar-p
in #14512. Fix #2 is also from #14512. Fix #4 and the
markRaw()additionto fix #1 are new here.