-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Intl.format slow performance when using PlainDate instead of Date #262
Comments
Please can you share your benchmarking code? |
Our main focus with the polyfill so far has been to ensure it's compliant to the Temporal spec. So we haven't spent a lot of time on perf optimizations, other than the most obvious ones like caching our usage of But if you find an optimization opportunity in the polyfill, PRs are always welcome! That said, seeing slower perf in this case isn't surprising. The polyfill's This conversion is a relatively expensive operation that runs hundreds of lines of JS code and that makes at least one additional call to Note that the There's also some one-time costs associated with the first use of some Temporal APIs, while the polyfill sets up internal caches (notably a single cached A native implementation will be a lot, lot faster: likely faster than But given the Temporal polyfill's dependency on native pre-Temporal If you have a particular, perf-sensitive use case that you want to share more details about, then we can probably recommend a faster way to achieve that use case that avoids expensive Here's a few techniques that could work:
Thanks for trying out this polyfill! |
#Thank you for the detailed information. Customers are ordering outside of germany in a different timezone but delivery is happening in germany. So I need to check if date in german timezone is sunday/saturday or if it is a holiday, so that I can disable these days. Also customer can for example not order for monday if current day (in germany) is a sunday. Thats why I need the timezone conversions. Previously I used my own simple
To speed up the current usage with
Is there any drawback doing this? Speeds (T = Temporal, D = Date): |
For getting the day of the week in zoned time, I would suggest not going through formatting at all! Formatting other than with function isWeekendInGermany(requestedDeliveryTime: Temporal.ZonedDateTime) {
const {dayOfWeek} = requestedDeliveryTime.withTimeZone('Europe/Berlin');
return dayOfWeek === 6 || dayOfWeek === 7;
} |
Tried that already and it is not significantly faster since the performance loss is from converting to the specific timezone. In my example PlainDate and Date both get converted to the timezone specific format, but PlainDate is a lot slower. |
I see, so the bottleneck is the internal One possible workaround, if you only need the An example of that is here, although that example has been languishing as an unmerged PR until I've finished some other higher-priority things so it might need some updating. That example is probably more comprehensive than you would need. You could probably cut out the parsing code, for example. |
Are you originally starting with a date like 2023-08-25 or an exact point in time like 2023-08-25T12:34:56Z? If the former, then the solution is very easy and fast: don't bother with time zones at all: pd = Temporal.PlainDate.from('2023-08-25');
console.log(pd.dayOfWeek);
// 5 You can probably shave a few tens of microseconds by using a cached calendar: isoCalendar = Temporal.Calendar.from('iso8601');
pd = Temporal.PlainDate.from('2023-08-25');
console.log(isoCalendar.dayOfWeek(pd));
// 5 But if you're starting with an exact instant (e.g. a const tzBerlin = Temporal.TimeZone.from('Europe/Berlin');
const isoCalendar = Temporal.Calendar.from('iso8601');
function isWeekendInGermany(instant) {
const dt = tzBerlin.getPlainDateTimeFor(instant);
const dayOfWeek = isoCalendar.dayOfWeek(dt);
return dayOfWeek === 6 || dayOfWeek === 7;
}
isWeekendInGermany(new Date().toTemporalInstant());
// => false
isWeekendInGermany(new Date().toTemporalInstant().add({hours: 24}));
// => true If that's still not fast enough for your use case, then the custom time zone that @ptomato recommends seems like your best bet. Although at that point, given that you already have a workable solution using Intl.DateTimeFormat, then I'd probably just stick with that one. Polyfills are, by definition, going to be slower than native solutions. All that said, a difference of 0.1-0.2 milliseconds is fairly small! Relative to all the other things that a system is doing, does the perf of this call significantly impact your app's overall performance? |
Oh wow I am so stupid ... obviously the 25.08.2023 is always a friday regardless of the timezone🙈 Time to take a break^^ It had some impact because I use it on a filter on a DateAdapter for Angular Material. When opening the year view, the days get checked to disable years if needed. But I also have some other additional performance issue here which I need to investigate. |
Cool. I'm going to close this issue because it sounds like the Temporal part is resolved. This was an interesting discussion. Thanks! |
When using
PlainDate
with a cachedIntl.DateTimeFormat
the performance is 15x worse than usingDate
.Is this expected because it is just a polyfill ?
Date:

PlainDate:

The text was updated successfully, but these errors were encountered: