Repository navigation
Skip shared-timer re-renders whose output cannot change - #397
Closed
mattcosta7 wants to merge 6 commits into
Closed
mattcosta7 wants to merge 6 commits into
mattcosta7 wants to merge 6 commits into
Conversation
One element under a minute old makes the shared timer tick every second, and every tick re-rendered every observed element. Each element now records when its relative text can next change and the inputs it was rendered from; ticks skip it until then unless an input changed. The tick schedule, rendered text, titles and events are unchanged. The same check skips the second render a newly inserted element got from its queued attribute update. update() still always re-renders. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1. dist/bundle.js threw "Cannot access "RelativeTimeElement" before initialization" on the first render. tsc (es2017) hoists a class's private methods into the computed `[refresh]` key, which runs while the class is being defined, and esbuild turns the class-name reference in `#recordRender` into an eager TDZ error. The prototype check now lives in a module-level `isPlainElement` that `#recordRender` and `[refresh]` share. dist/index.js was unaffected. test/bundle.js loads the built bundle, renders an element and ticks the shared timer. 2. `#stableFor` assumed a named zone's date can only change on a UTC quarter hour. Fixed offsets such as `+00:01` and historical offsets such as Africa/Monrovia's -00:44:30 break that, leaving "yesterday" showing after a fresh render says "2 days ago". Elements with a resolved time zone are no longer skipped. 3. With a clock before 1970, `now % msPerDay` is negative and the UTC midnight bound was too late. Such clocks are no longer skipped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…lang Replacing a RelativeTimeElement.prototype member, such as the date getter, before any element rendered still let elements skip, so a changed input went unrendered. Elements now skip only while the prototype has exactly the members it was defined with, checked once per tick and once per microtask checkpoint. The tests count renders through hasAttribute so they no longer change the class themselves. An invalid lang records a locale lookup that is never cached, so repeating it always failed and the element never skipped. Lookups the render left missing now repeat as missing, and force a full render if the entry has since been cached. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The class check was cached per task, so page code that replaced a prototype member mid-task, from an update listener during a tick or right after inserting an element, went unnoticed until the next task. It now checks the 20 members a render reads on every call (about 1 µs), with no cache. Whether an element is plain is checked when a render starts as well as when it records its inputs, so an override that removes itself while being read can't make the render look skippable. A render that can't be skipped now always clears #stableUntil, so a clock moving back can't reuse an older render's inputs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… timer reads Adding a member to RelativeTimeElement.prototype, such as a getAttribute that shadows the built-in, was not detected, as only the 20 members a render reads were compared. The prototype's full set of keys is now compared too. The shared timer read an element's date before checking for overrides, so an override that removed itself on that read went unnoticed; the check now comes first. The class check is reused only until page code may have run: the start of a tick, the start of each render and after its update event, each refresh outside a tick, and after any element with overrides. Within a tick of skipped elements it runs once. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…t writes The class check compared only the members a render reads, so replacing another one, such as connectedCallback, went unnoticed. It now compares every member's descriptor; the check is reused within a tick, so this costs about 3 µs once per tick. An instance override could change the prototype after calling the inherited update(), and page code run while a render writes the DOM, such as a removed custom element's disconnectedCallback, could change an input that was then recorded as rendered. Inputs are now read when a render starts, the render's own title write no longer counts as an attribute change, and page code is assumed to have run at the end of every render and after any element with overrides. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Closing. The core idea held up: independent differential checks matched
The guards for those cases made inserting elements about 35% slower, and made elements that never skip 30–60% slower per tick (measured). If this comes back, start from:
|
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.
Stacked on #396.
Problem
One element under a minute old makes the shared timer tick every second, and so does any running
format="elapsed"timer. Every tick callsupdate()on every observed element, even when none of their text can change. A page with 500 timestamps a few hours old and one fresh timestamp re-renders all 500 every second for that minute. With an elapsed timer it does so for as long as the timer runs.Inserted elements also render twice.
connectedCallbackrenders immediately, then the update queued byattributeChangedCallbackrenders again in a microtask.Change
After each render, an element records when its text can next change, plus the inputs it was rendered from. The shared timer and the queued attribute update skip the element until then, unless one of those inputs changed. Nothing observable changes:
relative-time-updatedfires at the same moments.Intlformatter caches end up the same.update()still always re-renders.The skip only applies to the default
relativeoutput withsecondprecision, for times between a minute and 28 days away. Within that rangeroundToSingleUnitcan only change the text when:thresholdElements with a
time-zone, on themselves or an ancestor, are never skipped. A fixed offset like+00:01can be any number of minutes, and historical offsets include seconds, so only anIntllookup could place that zone's midnight. Clocks before 1970 aren't skipped either. github.com sets notime-zone, so this exclusion doesn't affect it.Before skipping, the element checks that nothing else a render reads has changed:
lang,time-zoneandhour-cycleon ancestorsdata-prefers-absolute-timeon<html>and<body>languagechange#updateRenderRootContentchecks before writingIntl.DateTimeFormatandIntl.RelativeTimeFormatare still presentA render reads the inputs it records when it starts, before it writes to the DOM. So page code that runs while it writes, such as a removed custom element's
disconnectedCallback, or an update listener that changes an attribute or an ancestor, still triggers a re-render. The render's own title write doesn't count as an attribute change.A render's lookups in the shared
Intlformatter caches are recorded. A skip repeats them in the same order, so the least-recently-used order, and with it every later eviction, matches a full render. If an entry was evicted, the element renders in full. A lookup the render left missing, such as an invalidlanglocale that can't be constructed, repeats as missing, and forces a full render if the entry has appeared since.These always render as before:
time-zoneRelativeTimeElement.prototypediffers from how it was defined: any added, removed or replaced member, such as agetAttributethat would shadow the built-in, a replaceddategetter or a wrappedupdate(). The answer is reused only until page code may have run: the start of a tick, the start and end of each render and after its update event, each refresh outside a tick, and before and after any element with overrides. So changes made by update listeners, overrides or right after inserting an element are caught, and a tick of skipped elements checks once, for about 3 µs.duration,elapsedandmicroformatsno-titleelements, which fire an event on every renderWhat skipping assumes
Skipping assumes two things:
DateorIntlbuilt-ins. Changes toRelativeTimeElement.prototype, subclasses and per-instance overrides are detected.Results
Measured with Playwright in a page with 500 elements a few hours old and one element 2 seconds old, over 5 seconds of 1-second ticks:
update()callsupdate()A Chromium CPU profile of the whole tick, including the skip checks, drops from about 17.5 ms to about 9 ms per 8 seconds. A separate warm benchmark with 1,000 stable elements measured 4.6 ms → 1.3 ms per tick. Skipped elements still run
getUnitFactor, so the timer schedule is unchanged. They still read their ancestors and render root, so changes there are picked up on the same tick as before.Reading the host time zone costs one
Intl.DateTimeFormatconstruction per tick: about 23 µs in Chromium and 47 µs in WebKit. On pages with about 10 or fewer ticking elements, that roughly cancels the savings.Tests
In
test/relative-time.js, the suiteskipping re-renders that cannot change outputcovers four kinds of behavior.Differential tests. These tick the shared timer every simulated second (every 250 ms in one test). After each tick they compare every element with a freshly rendered copy that has the same attributes and ancestors. The windows cover:
tensePT45M30SandPT1H30Mtime-zoneand UTC midnights, including a local month endlang,time-zoneandhour-cycle, the absolute-time preference,languagechange, the shadow DOM, a custom title, and the clock going backwardsPerformance tests. 100 elements three hours old don't re-render while a fresh element ticks every second. An inserted element renders once. Both fail without this change.
API tests.
update()always re-renders, and an attribute change always re-renders.Built bundle.
test/bundle.jsloads the builtdist/bundle.js, renders an element and ticks the shared timer. The first version of this change threwCannot access "RelativeTimeElement" before initializationthere, because the bundle evaluates the computed[refresh]method key before the class binding exists. The source tests didn't catch it.Regression tests from four rounds of adversarial review and a ship-check. These cover:
no-titleeventsIntl.RelativeTimeFormatremoved at runtimequeueMicrotask+00:01andAfrica/Monrovia, and UTC midnight with a clock before 1970update()wrapped right after inserting an elementgetAttributeadded to the prototype, andconnectedCallback,attributeChangedCallbackor theonRelativeTimeUpdatedgetter replacedupdate()datetimewhile a render repairs the shadow rootlang(en_US) still rendering once and skippingThe tests count renders through
Element.prototype.hasAttribute('no-title'). Patchingupdate()would change the class and turn skipping off.Each of these fails without its fix.
test/intl-cache.tscovers recording and repeating lookups.To check that the tests catch mistakes, I broke the bound or the input checks in 16 ways before the time-zone change. Every one failed at least one test:
time-zoneelements aren't skipped)langignoredIntlpresence ignoredno-titleignoredThe suite takes about 20 seconds instead of 5.
npm test(build, lint, web-test-runner)npx prettier --check src test