From f88cc782864fa76075f81959389a2c887b98a904 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Tue, 4 Aug 2026 10:59:35 -0300 Subject: [PATCH] fix(perf): per-burst retry budget lifecycle, reset only on external render (#6717) --- static/ui.js | 23 ++- ...st_issue500_message_list_virtualization.py | 152 +++++++++++------- tests/test_issue734_message_windowing.py | 24 ++- 3 files changed, 135 insertions(+), 64 deletions(-) diff --git a/static/ui.js b/static/ui.js index 13583741..ee42cfbc 100644 --- a/static/ui.js +++ b/static/ui.js @@ -548,6 +548,7 @@ let _messageVirtualScrollRaf=0; let _messageVirtualWindowKey=''; let _messageVirtualMeasurementCycleKey=''; let _messageVirtualMeasurementRetryCount=0; +let _messageVirtualMeasurementBurstActive=false; let _messageVirtualScrollActive=false; let _messageVirtualScrollSettleTimer=0; let _messageVirtualDeferredMeasurement=null; @@ -595,6 +596,7 @@ function _clearMessageVirtualHeightCache(){ _messageVirtualWindowKey=''; _messageVirtualMeasurementCycleKey=''; _messageVirtualMeasurementRetryCount=0; + _messageVirtualMeasurementBurstActive=false; _messageVirtualScrollActive=false; clearTimeout(_messageVirtualScrollSettleTimer); _messageVirtualScrollSettleTimer=0; @@ -728,13 +730,32 @@ function _scheduleMessageVirtualMeasurementRefresh(windowMetrics){ if(_messageVirtualMeasurementCycleKey!==cycleKey){ _messageVirtualMeasurementCycleKey=cycleKey; } - if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS) return; + // Per-burst retry budget (#6717): the counter is preserved across ONLY the + // internally measurement-scheduled chain (requestAnimationFrame -> + // _scheduleMessageVirtualizedRender -> renderMessages -> re-measure -> here), + // so WebKit's A/B window-metric oscillation can never renew the budget and + // keep the rAF/measure loop alive (#6654). It is reset exactly once when a + // later EXTERNALLY initiated render begins a genuinely new measurement cycle + // (session load, message append, real content change) — never on cycle-key + // changes. A cycle-key change alone just records the new key above. + if(!_messageVirtualMeasurementBurstActive){ + _messageVirtualMeasurementRetryCount=0; + _messageVirtualMeasurementBurstActive=true; + } + if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS){ + // Budget exhausted: this burst is over (no further internal re-render is + // scheduled), so the next externally initiated cycle starts fresh instead + // of being starved of its own retries. + _messageVirtualMeasurementBurstActive=false; + return; + } _messageVirtualMeasurementRetryCount++; requestAnimationFrame(()=>{ _scheduleMessageVirtualizedRender(true); }); } function _markMessageVirtualMeasurementsSettled(windowMetrics){ _messageVirtualMeasurementCycleKey=_messageVirtualMeasurementCycleKeyFor(windowMetrics); _messageVirtualMeasurementRetryCount=0; + _messageVirtualMeasurementBurstActive=false; } function _messageVirtualHeightEntryMatches(previousEntry, nextEntry){ return !!( diff --git a/tests/test_issue500_message_list_virtualization.py b/tests/test_issue500_message_list_virtualization.py index 6bd963ca..994d2951 100644 --- a/tests/test_issue500_message_list_virtualization.py +++ b/tests/test_issue500_message_list_virtualization.py @@ -1112,6 +1112,7 @@ let _messageVirtualEstimatedRowHeight = 200; let _messageVirtualWindowKey = 'old-key'; let _messageVirtualMeasurementCycleKey = 'old-cycle'; let _messageVirtualMeasurementRetryCount = 3; +let _messageVirtualMeasurementBurstActive = false; const MESSAGE_VIRTUAL_DEFAULT_ROW_HEIGHT = 140; // #4367 introduced per-role default heights; the combined _clearMessageVirtualHeightCache // resets the estimate via _messageVirtualDefaultHeightForRole('default'), so the harness @@ -1146,81 +1147,116 @@ console.log(JSON.stringify({ def test_measurement_retry_budget_is_global_across_cycle_key_oscillation(): """#6654: _scheduleMessageVirtualMeasurementRefresh must NOT reset _messageVirtualMeasurementRetryCount when _messageVirtualMeasurementCycleKey - changes. If WebKit alternates between two measured window-metric sets - (A -> B -> A -> B), each key transition used to reset the two-render budget, - so the rAF/geometry-measurement loop never terminated (~30% WebContent CPU). - The MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS budget must cap the whole - convergence burst globally; it resets only when measurement settles - (_markMessageVirtualMeasurementsSettled) or the cache/session resets - (_clearMessageVirtualHeightCache).""" + changes INSIDE one measurement burst. If WebKit alternates between two + measured window-metric sets (A -> B -> A -> B), each key transition used to + reset the two-render budget, so the rAF/geometry-measurement loop never + terminated (~30% WebContent CPU). The MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS + budget must cap the whole convergence burst; the burst ends at the cap (or + when measurement settles), so the NEXT externally initiated cycle gets a + fresh budget. The harness drives the thrash causally through the internal + chain (each scheduled render re-measures the next oscillating key).""" js = UI_JS_PATH.read_text(encoding="utf-8") source = _extract_func_script(js) + """ let scheduled = 0; let _messageVirtualMeasurementCycleKey = ''; let _messageVirtualMeasurementRetryCount = 0; +let _messageVirtualMeasurementBurstActive = false; let _messageVirtualScrollActive = false; let _messageVirtualDeferredMeasurement = null; const MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS = 2; function _messageVirtualMeasurementCycleKeyFor(w) { return w.key; } -function _scheduleMessageVirtualizedRender(force) { scheduled++; } -function requestAnimationFrame(cb) { cb(); } -eval(extractFunc('_scheduleMessageVirtualMeasurementRefresh')); -// WebKit alternates between two adjacent window-metric keys across -// getBoundingClientRect passes. Before the fix every key transition reset the -// budget, so all four passes scheduled a re-render. -for (const key of ['A','B','A','B']) { - _scheduleMessageVirtualMeasurementRefresh({ key: key }); +// Drive the burst causally: each internally scheduled render re-measures and +// re-enters the scheduler with the next WebKit-oscillated key. +let keys = ['B','A','B','A']; +let idx = 0; +function _scheduleMessageVirtualizedRender(force){ + scheduled++; + if(idxB->A->B. _scheduleMessageVirtualMeasurementRefresh({ key: 'A' }); console.log(JSON.stringify({ scheduled: scheduled, retries: _messageVirtualMeasurementRetryCount, + burstActive: _messageVirtualMeasurementBurstActive, })); """ metrics = json.loads(_run_node(source)) - assert metrics["scheduled"] == 3, ( - "stable-key burst must stop at MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS " - f"and resume only after a budget reset: scheduled {metrics['scheduled']} " - "(expected 3: 2 capped + 1 after explicit reset)" + assert metrics["scheduled"] == 2, ( + "cycle-key oscillation must not reset the retry budget within a burst: " + f"scheduled {metrics['scheduled']} re-renders (expected 2, the " + "MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS cap for the whole burst)" ) - assert metrics["retries"] == 1, ( - f"retry count after explicit reset must be 1, got {metrics['retries']}" + assert metrics["retries"] == 2, ( + "retry count must keep accumulating across key changes within the burst: " + f"ended at {metrics['retries']} (expected 2)" + ) + assert metrics["burstActive"] is False, ( + "the burst must end once the budget is exhausted so the next externally " + f"initiated cycle can reset it: burstActive {metrics['burstActive']}" + ) + + +def test_external_measurement_after_thrash_gets_fresh_retry_budget(): + """#6717 re-gate: exhausting the budget with the WebKit A/B thrash must NOT + starve a genuinely new externally initiated measurement cycle. Drive the + A/B oscillation causally until the per-burst budget is exhausted, then start + an independent changed-C measurement WITHOUT touching private state: + (a) C must still receive its own retries, and (b) C's own internal + oscillation must remain capped at MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS.""" + js = UI_JS_PATH.read_text(encoding="utf-8") + source = _extract_func_script(js) + """ +let scheduled = 0; +let _messageVirtualMeasurementCycleKey = ''; +let _messageVirtualMeasurementRetryCount = 0; +let _messageVirtualMeasurementBurstActive = false; +let _messageVirtualScrollActive = false; +let _messageVirtualDeferredMeasurement = null; +const MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS = 2; +function _messageVirtualMeasurementCycleKeyFor(w) { return w.key; } +let keys = []; +let idx = 0; +function _scheduleMessageVirtualizedRender(force){ + scheduled++; + if(idx=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS) return;" in UI_JS + assert "if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS){" in UI_JS assert "_scheduleMessageVirtualMeasurementRefresh(virtualWindow);" in UI_JS assert "_markMessageVirtualMeasurementsSettled(virtualWindow);" in UI_JS def test_measurement_retry_budget_not_reset_by_cycle_key_change(): - """#6654: the cycle-key branch of _scheduleMessageVirtualMeasurementRefresh + """#6654/#6717: the cycle-key branch of _scheduleMessageVirtualMeasurementRefresh must record the new key but never reset _messageVirtualMeasurementRetryCount. Resetting on key change lets WebKit's A->B->A->B metric oscillation renew - the two-render budget forever, keeping the rAF/measure loop alive.""" + the two-render budget forever, keeping the rAF/measure loop alive. The ONLY + reset allowed inside the scheduler is the per-burst one: when a NEW + externally initiated cycle begins (no burst active), guarded by + if(!_messageVirtualMeasurementBurstActive).""" idx = UI_JS.index("function _scheduleMessageVirtualMeasurementRefresh(windowMetrics)") end = UI_JS.index("function _markMessageVirtualMeasurementsSettled", idx) body = UI_JS[idx:end] # The key-change branch must only assign the key. assert "if(_messageVirtualMeasurementCycleKey!==cycleKey){" in body - assert "_messageVirtualMeasurementRetryCount=0;" not in body, ( + key_branch_start = body.index("if(_messageVirtualMeasurementCycleKey!==cycleKey){") + key_branch_end = body.index("}", key_branch_start) + key_branch = body[key_branch_start:key_branch_end] + assert "_messageVirtualMeasurementRetryCount=0;" not in key_branch, ( "cycle-key change must not reset the retry budget (issue #6654)" ) + # The per-burst reset is allowed only at the start of a new external cycle. + assert "if(!_messageVirtualMeasurementBurstActive){" in body + guard_pos = body.index("if(!_messageVirtualMeasurementBurstActive){") + reset_pos = body.index("_messageVirtualMeasurementRetryCount=0;") + assert guard_pos < reset_pos, ( + "the budget reset must be guarded by the burst-active check " + "(per-burst lifecycle, issue #6717)" + ) # The budget guard and increment must still be present. - assert "if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS) return;" in body + assert "if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS){" in body assert "_messageVirtualMeasurementRetryCount++;" in body