dayjs icon indicating copy to clipboard operation
dayjs copied to clipboard

Cache toLocaleString

Open janousek opened this issue 5 years ago • 5 comments

Calls to "toLocaleString" in the timezone plugin is extremly slow: https://github.com/iamkun/dayjs/blob/ed9629b5ab2895652111fc854e6081422ed5c010/src/plugin/timezone/index.js#L99

The same problem was discussed in luxon: https://github.com/moment/luxon/issues/352

The solution for this performance problem is caching like they did in luxon: https://github.com/moment/luxon/commit/bb77d5e6b968b0a2cc1be72d295f40a43a6687fb

Just use Intl.DateTimeFormat instead of "toLocaleString" (it always produce the equivalent result) and cache these instances of Intl.DateTimeFormat.

janousek avatar Nov 22 '20 13:11 janousek

I don't know it's same problem or not. When we replace 'moment-timezone' with 'dayjs.tz', it's very slow.

Here is performance

origin_1ig9898mvwcu74a2sg9ntuhbb0orub97eiaiyud20nwdh2sjcpa origin_1k0lnmv1pvb3uw3suw6i7umzyzfvfpe33fmrtl7jhnzi0lf6d5c origin_7t19awy843emvs26lk82l5pliqk2nxbppens44r571r7zctfv origin_7tn2zs00hdkh0p5wv3pd7co78s9x0e67zneej6adwljt9i2a9

All point to proto.tz

defghy avatar Dec 21 '21 02:12 defghy

Hi, it seems this issue does not have any visibility. We are observing the same damming performance on our end, and it is starting to become an issue. image

We might move away from the timezone plugin for some of our hot functions, but I'd be happy to contribute back to a package that we use so extensively.

It seems like the timezone plugin is the only place in the codebase where there's still usage of toLocaleString(). I'd be keen on submitting a PR that would implement the solution presented by @janousek , but I'm afraid I am unsure of how dayjs manages caching. If anyone can point me to some file or relevant function, I'd be happy to give it a try.

half-shell avatar Apr 21 '22 10:04 half-shell

This code seems to have landed in a different form on 1.11.5? https://github.com/iamkun/dayjs/blob/dev/src/plugin/timezone/index.js#L12

Now, we continued to have issues even with that version because const target = date.toLocaleString('en-US', { timeZone: timezone })

ends up initializing a new version of Intl.DateTimeFormat each time, per MDN docs

We changed that with the following for better perf results: const target = getDateTimeFormat(timezone).format(date);

nuria avatar Sep 29 '22 22:09 nuria

@nuria how did you expose getDateTimeFormat since it isn't exported, was it just forked or copied?

billoneil avatar Oct 11 '24 20:10 billoneil

I have encountered this problem too. Is there any updates?

Image

acrazing avatar May 19 '25 17:05 acrazing