Skip to content

Should we be caching the ICU4X dates? #505

Description

@Manishearth

Almost any Temporal object will contain an ISODate, and when you access it for calendared information, it will construct a Date<AnyCalendar> on the fly via Calendar::from_iso().

This seems inefficient: 99% of the time we have constructed a date we already have the from-ISO value; and even if we don't, most useful methods on Date need to call this. In some cases we can rely on the optimizer to inline and collapse consecutive from_iso() calls, but that won't work e.g. over FFI.

We should benchmark some of this, too.

Useful workloads:

  • Constructing a date (via various methods)
  • Calling .to_ixdtf_string()
  • Calling a single calendared getter on a date
  • Calling most calendared getters on a date, e.g. for formatting

Activity

  1. sffc commented on Aug 11, 2025

    @sffc
    Contributor

    We've designed ICU4X to optimize for from_iso being an efficient operation. The only inefficiency left are the far-away Chinese and Dangi dates, but we have a path forward to resolve those (unicode-org/icu4x#5778). We also have a path forward to improve fixed-to-iso which is another operation that will be in the fast path (Neri), which I think this project already implements.

  2. nekevss commented on Aug 11, 2025

    @nekevss
    Member

    Continuously constructing ICU4X dates has always been a concern, so I'd definitely be intrigued about caching them. We should also definitely start setting up some benchmarks for this and for Duration.

  3. Manishearth commented on Aug 12, 2025

    @Manishearth
    ContributorAuthor

    @sffc Yeah, though I do wonder if there's still a cost of from_iso being called 10-12 times when it comes to formatting.

    Also worried about timezone lookup costs for ZDT in that case.

    Should be fine, we should perhaps benchmark it a bit and monitor.

  4. sffc commented on Aug 12, 2025

    @sffc
    Contributor

    @sffc Yeah, though I do wonder if there's still a cost of from_iso being called 10-12 times when it comes to formatting.

    Context? That shouldn't need to happen; it's not observable

  5. Manishearth commented on Aug 12, 2025

    @Manishearth
    ContributorAuthor

    Ah, sorry, should have been clearer. Currently formatting goes through get_epoch_ms(), as requested by the spec, but if/when ICU4X and ICU4C have divergences1 we'll need to shuttle across all the fields, which involves calling all of the .year() etc methods in sequence. That won't inline across FFI.

    In theory we can create a FormattableDate struct over FFI and hope that CSE optimizations batch up the from_iso.

    So there are other options for us here.

    Footnotes

    1. And they likely will once we do proleptic approximations! They already diverge for rgsa. ↩

  6. sffc commented on Aug 12, 2025

    @sffc
    Contributor

    An ffi::Date wraps a Date<AnyCalendar>, so when formatting, you should be able to convert your epoch time* to a calendar date once, and then read all the fields.

    * It shouldn't be observable that the date goes through epoch time. That's just a technique the spec authors used in order to flatten all the Temporal types to a single type, and they picked epoch time since Intl.DateTimeFormat was already wired up to use epoch time.

  7. Manishearth commented on Aug 13, 2025

    @Manishearth
    ContributorAuthor

    @sffc what's ffi::Date? Are you talking about ICU4X Dates? I don't think our plan for handling ICU4X/ICU4C version skew currently involves pulling in ICU4X FFI (since we have other diplomat version problems to solve there). The hope was to reuse Temporal types.

  8. sffc commented on Aug 13, 2025

    @sffc
    Contributor

    OK, sure, in the temporal_rs FFI, you would either need to return an ICU4X ffi::Date or else return your own little wrapper type thing for formatting, which now I see is what you were probably thinking when you said FormattableDate.

  9. Manishearth commented on Aug 18, 2025

    @Manishearth
    ContributorAuthor

    Actually, I think this matters more for ZonedDateTime.

    ZonedDateTime stores a UTC timestamp (as requested by the spec), and unconditionally calls self.tz.get_iso_datetime_for(&self.instant, provider) for most operations. Those are timezone computations that can be somewhat expensive depending on the backend.

    We should either be caching a UtcOffset, or caching a full IsoDateTime.

  10. Manishearth commented on Aug 18, 2025

    @Manishearth
    ContributorAuthor

    The added benefit is that ZDT no longer needs a provider for all of its non-constructing methods. Yay!

  11. sffc commented on Aug 18, 2025

    @sffc
    Contributor

    You should definitely store at least a UtcOffset in ZonedDateTime.

  12. added a commit that references this issue on Aug 20, 2025
  13. Manishearth commented on Aug 26, 2025

    @Manishearth
    ContributorAuthor

    We cached offsets in #510

    I think the general consensus here is to not cache dates for now. Can reopen if it's a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions