Performance problem with DateTimeFormat polyfill

[andreialecu]

Which package?

It appears that using the DateTimeFormat polyfill to format dates can be extremely slow, especially on lower end Android devices.

We measured a single DateTimeFormat to 20ms on a real device. This blocks the main thread resulting in major performance problems.

Originally reported in: facebook/hermes#23 (comment) and comments around it.

Describe the bug

Not really a bug, but perhaps performance could be improved.

To Reproduce

delete Intl.getCanonicalLocales;
delete Intl.Locale;
delete Intl.PluralRules;
delete Intl.NumberFormat;
delete Intl.DateTimeFormat;

// Polyfills required to use Intl with Hermes engine
require("@formatjs/intl-getcanonicallocales/polyfill");
require("@formatjs/intl-locale/polyfill");
require("@formatjs/intl-pluralrules/polyfill");
require("@formatjs/intl-pluralrules/locale-data/en");
require("@formatjs/intl-numberformat/polyfill");
require("@formatjs/intl-numberformat/locale-data/en");
require("@formatjs/intl-datetimeformat/polyfill");
require("@formatjs/intl-datetimeformat/locale-data/en");
require("@formatjs/intl-datetimeformat/add-golden-tz");

for (let y = 0; y < 5; y++) {
  const start = Date.now();
  const dtf = new Intl.DateTimeFormat("en");
  for (let i = 0; i < 100; i++) {
    dtf.format(new Date());
  }
  console.log(Date.now() - start);
}

Run the above script with node --cpu-prof test.js to see a cpu trace.

Codesandbox URL

https://codesandbox.io/s/determined-nash-j9wug?file=/src/index.js

Expected behavior

DateTimeFormat polyfill should not be so slow.

Screenshots

Screenshot of profile session when using nodejs (slow paths can be seen there as well):

Additional context

Takes around 0.2-0.5ms per format on an M1 CPU which has among the best single threaded performance in the world right now. Takes around 20ms on a mid level android CPU.

[andreialecu]

One thing to note is that many libraries (such as luxon), will initialize a new DateTimeFormat instance on every format.

If the constructor call is moved inside the loop in the example above, the time per format doubles. Caching, or improving the DateTimeFormat constructor's performance would also be great, if possible.

Profiling output when also constructing DateTimeFormat instances:

[andreialecu]

Been poking at this out of curiosity.

It appears that there's a complex implementation here:

formatjs/packages/ecma402-abstract/262.ts

Lines 204 to 211 in 4324002

	export function YearFromTime(t: number) {
	const min = Math.ceil(t / MS_PER_DAY / 366)
	let y = min
	while (TimeFromYear(y) <= t) {
	y++
	}
	return y - 1
	}

Simply using new Date(t).getFullYear() results in a 40% speed up.

Before: 10,731 ops/sec ±0.92% (92 runs sampled)
After: 14,115 ops/sec ±1.33% (91 runs sampled)

Not sure why that logic is used instead of using the native Date.

DayFromYear could also be replaced with simply Date.UTC(year) / MS_PER_DAY for another modest speed-up.

[andreialecu]

formatjs/packages/ecma402-abstract/DisplayNames/CanonicalCodeForDisplayNames.ts

Lines 11 to 13 in 78d3558

	function isUnicodeScriptSubtag(script: string): boolean {
	return ALPHA_4.test(script)
	}

It appears the above function is called with a parameter of undefined a majority of the time. Adding a check like return script && ALPHA_4.test(script); results in another good speed-up:

15,100 ops/sec ±1.12% (94 runs sampled)

[andreialecu]

Public repository with the patches mentioned above if a maintainer wants to take a look:
https://github.com/andreialecu/formatjs-datetimeformat-perf

Currently about 100% slower without the patches.

I think it can be made much much faster with memoization or other improvements. Most of the time is spent creating the same type of locale.

One example is that it calls removeLikelySubtags('en') unnecessarily, which does a lot of work to only return en again.

If I make that function simply return tag (would be incorrect in the general sense, but it seems ok here), the performance increases further by around 20%: 17,782 ops/sec ±1.06% (91 runs sampled)

The original, non patched version only does 9k ops per second, so it's a 100% speed-up with only a few minor changes.

[longlho]

Using new Date(t).getFullYear() would be wrong because Date is timezone-sensitive. As for DayFromYear it takes into account leap year for Gregorian calendar.
We don't memoize simply because that'd just be an unbounded memory leak. Most of those operations are done within the constructor so it's a common practice to memoize that when you use it.

The bottom line is that performance isn't a priority for us tbh since i18n is complex enough to be implemented correctly. This in combination with the fact that you have to strictly follow the spec to prevent security issues like object pollution or cross-Realm pollution.