perf: cache Intl formatters instead of constructing per render#364
Open
mattcosta7 wants to merge 2 commits into
Open
perf: cache Intl formatters instead of constructing per render#364mattcosta7 wants to merge 2 commits into
mattcosta7 wants to merge 2 commits into
Conversation
Constructing Intl.*Format is expensive (often more than format() itself), yet update() built fresh DateTimeFormat/RelativeTimeFormat/Locale instances on every tick, and the DurationFormat ponyfill built a NumberFormat per unit plus a ListFormat per format. On pages with many <relative-time> elements this dominates tick cost. Add a shared src/intl-cache.ts memoizing DateTimeFormat, RelativeTimeFormat, NumberFormat, and Locale by locale+options, and route every formatter path through it: all formatters in relative-time-element.ts, NumberFormat/ListFormat in duration-format-ponyfill.ts, and DurationFormat instances in duration.ts. The 12h-cycle probe is memoized once. Formatters are immutable/stateless so sharing across elements is safe.
The #lang, timeZone, and hourCycle getters each walk ancestors via closest(), and were read by multiple formatters per tick. Resolve them a single time at the top of update() and thread them into the formatting helpers, avoiding repeated DOM traversal per render.
4a0a9b0 to
a39a74d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Caches Intl formatters to reduce per-tick rendering costs, building on #363.
Changes:
- Adds shared formatter and locale caches.
- Reuses duration, number, and list formatters.
- Resolves inherited formatting settings once per update and adds cache tests.
Show a summary per file
| File | Description |
|---|---|
src/intl-cache.ts |
Adds shared Intl caches. |
src/relative-time-element.ts |
Routes rendering through cached formatters. |
src/duration.ts |
Caches duration formatters. |
src/duration-format-ponyfill.ts |
Caches number and list formatters. |
test/intl-cache.ts |
Tests cache reuse and key distinctions. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 6
- Review effort level: Medium
| const lang = this.closest('[lang]')?.getAttribute('lang') || this.ownerDocument.documentElement.getAttribute('lang') | ||
| try { | ||
| return new Intl.Locale(lang ?? '').toString() | ||
| return intlLocale(lang ?? '').toString() |
|
|
||
| toLocaleString(locale: string, opts: DurationFormatOptions) { | ||
| return new DurationFormat(locale, opts).format(this) | ||
| const key = `${locale}\u0000${JSON.stringify(opts)}` |
Comment on lines
+10
to
+11
| // sharing is safe. The number of distinct combinations on a page is small and | ||
| // bounded, so a plain `Map` is sufficient and needs no eviction. |
|
|
||
| // `DurationFormat` normalizes its options on construction, so reuse one instance | ||
| // per (locale, options) combination rather than rebuilding it on every format. | ||
| const durationFormats = new Map<string, DurationFormat>() |
| ListFormatPonyFill) as {new (locale?: string, options?: {type: string; style: string}): ListFormatter} | ||
|
|
||
| // Reuse one list formatter per (locale, options) combination. | ||
| const listFormats = new Map<string, ListFormatter>() |
Comment on lines
+23
to
+25
| const key = cacheKey(locale, options) | ||
| let format = dateTimeFormats.get(key) | ||
| if (!format) dateTimeFormats.set(key, (format = new Intl.DateTimeFormat(locale, options))) |
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
Constructing an
Intl.*Formatis expensive — frequently more costly than the subsequent.format()call. Yetupdate()builds fresh formatters on every tick, and a page can render hundreds of<relative-time>elements, so this dominates per-tick cost.Per render today, an element could construct:
Intl.DateTimeFormatfor the title, and again for datetime /#isToday/#isCurrentYear/ absolute-time pathsIntl.RelativeTimeFormatfor relative/micro/absolute formattingIntl.Localeon every#langaccess (and#langis read once per formatter)NumberFormatper unit (up to 8) plus aListFormatperformat()DurationFormatpertoLocaleStringChanges
src/intl-cache.tsmemoizingDateTimeFormat,RelativeTimeFormat,NumberFormat, andLocalebylocale+JSON.stringify(options). Instances are shared across all elements. Formatters are immutable/stateless, so sharing is safe; the set of distinct (locale, options) combos on a page is small and bounded, so a plainMapneeds no eviction.DateTimeFormat/RelativeTimeFormatsites +Intl.Localeinrelative-time-element.ts,NumberFormat/ListFormatinduration-format-ponyfill.ts, andDurationFormatinduration.ts.isBrowser12hCycle).#lang/timeZone/hourCycleonce perupdate()(each is aclosest()DOM walk read by several formatters per tick) and thread them into the formatting helpers.Tests
Added
test/intl-cache.tsasserting instance reuse for identical locale+options, distinct instances for differing options/locales, and thatundefinedoption values key the same as absent ones. Full suite passes in Chromium.Notes
perf/skip-noop-render-writesbranch, so its diff shows only the formatter changes. Merge perf: skip redundant render-root and title writes on no-op ticks #363 first, then this fast-forwards with no conflict.undefinedoption values (e.g. an absenttimeZone) are dropped byJSON.stringify, matching the behavior of passing them explicitly — verified by test.