kotlinx-datetime icon indicating copy to clipboard operation
kotlinx-datetime copied to clipboard

FR: Use inline class for Instant

Open bubenheimer opened this issue 3 years ago • 12 comments

When initially looking at this library I was hoping to find an implementation of Instant that just wraps a Long or Double with an inline class, similar to the implementation of Duration. I would like to replace plain Long millisecond values in web server code where I cannot compromise on the memory footprint needed for these values. My API needs are basic, essentially "ms since epoch" (or possibly: ms since an arbitrary more recent reference point), adding/subtracting Durations, getting time deltas as Duration, plus basic logging for debug & monitoring purposes.

bubenheimer avatar May 27 '21 04:05 bubenheimer

ms since an arbitrary more recent reference point

"Arbitrary" as in chosen by us, the library writers, or by you? If it's the latter, then Duration is a much better fit for your use case than Instant, which can be thought of as a Duration whose beginning is fixed to be the start of the epoch.

Also, Duration values can be added to and subtracted from each other, so it seems like all the needs you listed (aside from logging) are met by Duration.

dkhalanskyjb avatar May 27 '21 12:05 dkhalanskyjb

Thanks, those are valid points regarding the more hypothetical "relative time" use case, I did mean arbitrary reference point chosen by my own code. On that note, if I did decide to go down this route at some point, I would want to use Int as the underlying base type for millisecond-precision durations up to about 49 days, which is sufficient for my use case. I cannot do this with the Duration type.

My primary use case at this time remains a Long/Double-wrapping inline Instant class; the alternative relative time option is a somewhat radical optimization to my present codebase, so I'm not planning to pursue it any time soon, if ever.

As a stopgap I've created my own CompactInstant inline wrapper class around a msSinceEpoch Long for now, but I'm still pulling in kotlinx-datetime's Instant class for a human-readable toString() in a multiplatform environment. I'm still interested in a genuine kotlinx-datetime CompactInstant implementation.

bubenheimer avatar May 27 '21 13:05 bubenheimer

[...] I would want to use Int as the underlying base type for millisecond-precision durations up to about 49 days, which is sufficient for my use case. I cannot do this with the Duration type.

Could you explain why not? Duration wraps a Long and supports far more than 49 days in millisecond precision.

I've created my own CompactInstant inline wrapper class around a msSinceEpoch Long for now

But how is it different from a Duration, except for toString() implementation which could be added under a different name as an extension function for Duration?

dkhalanskyjb avatar May 27 '21 13:05 dkhalanskyjb

Long vs. Int: the motivation for this feature request is memory footprint. Int has half the memory footprint of Duration's Long (on many platforms). My use case is limited to durations of no more than one or two days, and I do not need nanosecond precision, so Int would be a good choice.

Using a CompactInstant type to represent absolute points in time is much easier to work with than Duration; the whole programming world revolves around msSinceEpoch, the effort of departing from this model on my own is difficult to contain in a solo project - it would create significant additional code complexity plus maintenance burdens and make code less reusable. In particular, all relative timepoints now need a reference point to be valid, and this reference point cannot be stored in or referenced from a duration inline class. The reference point marks the start of a user session and remains valid for the duration of the session, but each session has a different reference point, so I'd have to carry around the reference point to wherever I need to transform the relative duration to an absolute timepoint.

bubenheimer avatar May 27 '21 14:05 bubenheimer

We'd rather avoid introducing another type for representing instants into kotlinx-datetime because doing so would not only inflate the API surface significantly, but I believe would also make using the library harder: the users would constantly have to decide whether they should use Instant or CompactInstant in their code.

ilya-g avatar Jul 02 '21 15:07 ilya-g

Could it make sense to provide some of the general-purpose date/time APIs for usage with epoch-based milliseconds without the extra step of a throwaway CompactInstant conversion? And keep those APIs in some separate "core" library artifact, so it wouldn't be necessary to introduce the full datetime dependencies when almost none of it is used?

In my KMP project I have to pull in kotlinx-datetime just to log some human-readable debug timestamps from a ms-since-epoch value; alternatively I could introduce platform-specific dependencies, but that seems even worse.

bubenheimer avatar Jul 12 '21 00:07 bubenheimer

@bubenheimer https://github.com/korlibs/klock has what you are looking for.

OrhanTozan avatar Nov 25 '21 12:11 OrhanTozan

@OrhanTozan, are you talking about the DateTime class there https://github.com/korlibs/klock/blob/master/klock/src/commonMain/kotlin/com/soywiz/klock/DateTime.kt? Doesn't look like it supports working with Duration, which is more or less the only feature requested here.

dkhalanskyjb avatar Nov 25 '21 12:11 dkhalanskyjb

@bubenheimer, it looks like your needs can be easily satisfied with just a tiny wrapper around Duration that only calls our library for pretty printing. Allocating an Instant for printing should not be a big deal, given how the resulting string takes up even more bytes.

@JvmInline
public value class CompactInstant(public val sinceEpoch: Duration): Comparable<CompactInstant> {

    public companion object {
        public fun fromEpochMilliseconds(epochMilliseconds: Long): CompactInstant =
            CompactInstant(epochMilliseconds.milliseconds)
    }

    private val kotlinxInstant: Instant
        get() = Instant.fromEpochMilliseconds(0) + sinceEpoch

    override fun toString(): String = kotlinxInstant.toString()

    public operator fun plus(other: Duration): CompactInstant = CompactInstant(sinceEpoch.plus(other))

    public operator fun minus(other: Duration): CompactInstant = CompactInstant(sinceEpoch.minus(other))

    public operator fun minus(other: CompactInstant): Duration = sinceEpoch.minus(other.sinceEpoch)

    override fun compareTo(other: CompactInstant): Int = sinceEpoch.compareTo(other.sinceEpoch)
}

Tested like this:

        val instant1 = CompactInstant.fromEpochMilliseconds(1637843185123L)
        val instant2 = CompactInstant.fromEpochMilliseconds(1444443185123L)
        assertEquals("2021-11-25T12:26:25.123Z", instant1.toString())
        assertEquals("2015-10-10T02:13:05.123Z", instant2.toString())
        assertTrue(instant2 < instant1)
        assertFalse(instant2 > instant1)
        assertNotEquals(instant2, instant1)
        assertTrue(instant2 != instant1)
        assertEquals(instant2, instant2)

In my opinion, if something in its entirety takes about 20 straightforward lines of code, it's better to just copy-paste the definition into the project and not include any library: this way, adding new operations, removing old ones, and in general iterating on the thing is more straightforward. EDIT: for example, for eventually replacing the Long duration with an Int. :)

What do you think?

dkhalanskyjb avatar Nov 25 '21 12:11 dkhalanskyjb

Missed another portion of your request.

Could it make sense to provide some of the general-purpose date/time APIs for usage with epoch-based milliseconds without the extra step of a throwaway CompactInstant conversion?

Do you mean, just a separate library artifact that only contained a function to convert the number of epoch milliseconds into an ISO-8601 string? All the other things you've requested are more or less just + and - operations.

For formatting, Instant internally uses LocalDateTime, which, in turn, uses LocalDate. So, in fact, a good chunk of the library is being used just for this string conversion.

dkhalanskyjb avatar Nov 25 '21 12:11 dkhalanskyjb

@OrhanTozan thank you for the pointer.

@dkhalanskyjb:

Do you mean, just a separate library artifact that only contained a function to convert the number of epoch milliseconds into an ISO-8601 string? All the other things you've requested are more or less just + and - operations.

Yes, I was thinking of it as foundational functionality (a core artifact or utility artifact), that other datetime artifacts would use. But from what you're saying that's not how the code is structured, unless it would be appropriate to move LocalDateTime & LocalDate into this core artifact. All I want here is write a human-readable date/time string to Android logcat and/or a bug tracker for troubleshooting; it's hard to justify pulling in the entire datetime library only for this, but I can't easily and automatically post-process those types of logs, either.

Also, thank you for coming up with that code gist, it is interesting to see the Duration-based wrapper, I may switch to it at some point.

bubenheimer avatar Dec 02 '21 07:12 bubenheimer

My needs for Java-agnostic datetime handling may expand over time, that would provide reasonable justification for incorporating the full datetime artificat.

bubenheimer avatar Dec 02 '21 07:12 bubenheimer