Poko icon indicating copy to clipboard operation
Poko copied to clipboard

Cache computed `hashCode` and `toString` for deeply immutable types

Open drewhamilton opened this issue 4 years ago • 4 comments

Idea from @ZacSweers. Similar to AutoValue's @Memoized annotation.

If a Poko class is deeply immutable—i.e. all of its members are vals and are also deeply immutable—then there is no need to repeatedly re-calculate hashCode and toString results after their first call.

I like the feature but have a few concerns before moving forward:

  • [x] Kotlin data classes don't have this feature. Do I want to add features that data classes don't have?
  • [ ] How do I want to approach adding multiple opt-in features: Does each new opt-in feature get a new annotation? Does each new annotation get a corresponding customization Gradle property? It's easy to imagine this getting out of hand. (Also applies to #50.) -- Possible alternative to adding new annotations would be a "mode" for Poko to operate in as a whole—but then how does it know which classes are deeply immutable?
  • [ ] Is it possible to have mutually exclusive opt-in features? How can I protect against that?

drewhamilton avatar Dec 04 '21 19:12 drewhamilton

I'm definitely very resistant to anything that is automatic or inferred. This feature adds to the object's memory size. Sometimes you can squeeze in an extra field for free (thanks to 8-byte alignment), or sometimes can end up adding 8-bytes to each object for a single 4-byte cache. For objects which could be allocated millions of times that memory impact could be significant so opting-in seems wise (plus the whole how-do-you-know problem).

It's possible to copy the design of AutoValue but instead using super calls instead of their abstract.

@Poko class Test(
  val name: String,
) {
    @Memoized
    override fun toString() = super.toString()
}

One downside to this design is that a do-nothing override is otherwise a good design candidate for #50.

@Poko class Test(
  val name: String,
) {
    override fun toString() = super.toString()
}

Having an explicit override disable toString generation, but then annotating it both re-enable generation and perform memoization would be very confusing.

JakeWharton avatar Aug 07 '23 19:08 JakeWharton

a do-nothing override is otherwise a good design candidate for #50.

Explicit overrides are supported, so this should be viable today without Poko adding anything. If Poko were to implement explicit support for #50 then it'd have to be more concise, otherwise there's no point.

@Memoized
override fun toString() = super.toString()

It's nice for readability that it's directly on the affected function, but I dislike that it's lying about calling super – it's just too unintuitive I think.

It's tempting to pursue a version that doesn't lie:

@Memoized
override fun toString()

But inventing a new type of declaration feels dubious. It may be technically possible with FIR plugins though.

drewhamilton avatar Aug 08 '23 00:08 drewhamilton

The runtime could ship marker functions like

@Memoized
override fun toString() = pokoToString()

to get close to that today. The functions would throw under the assumption that they'll always be replaced by the compiler plugin (cite does this for example).

Will be interesting to see if you can accomplish your proposal, though.

JakeWharton avatar Aug 08 '23 00:08 JakeWharton

If caching is done the obvious way that everyone does it (private field, if check against default value in the function), the field needs to be @Volatile for Kotlin/Native.

See https://github.com/Kotlin/kotlinx-io/pull/208.

JakeWharton avatar Aug 09 '23 17:08 JakeWharton