Skip to content
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

Cache computed hashCode and toString for deeply immutable types #67

Open
1 of 3 tasks
drewhamilton opened this issue Dec 4, 2021 · 4 comments
Open
1 of 3 tasks
Labels
enhancement New feature or request

Comments

@drewhamilton
Copy link
Owner

drewhamilton commented Dec 4, 2021

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:

  • 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 Option to omit toString generation #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 drewhamilton added the enhancement New feature or request label Dec 4, 2021
@JakeWharton
Copy link
Collaborator

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.

@drewhamilton
Copy link
Owner Author

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.

@JakeWharton
Copy link
Collaborator

JakeWharton commented Aug 8, 2023

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
Copy link
Collaborator

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 Kotlin/kotlinx-io#208.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants