-
Notifications
You must be signed in to change notification settings - Fork 12
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
Comments
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 @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. |
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.
It's nice for readability that it's directly on the affected function, but I dislike that it's lying about calling 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. |
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. |
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 |
Idea from @ZacSweers. Similar to AutoValue's
@Memoized
annotation.If a Poko class is deeply immutable—i.e. all of its members are
val
s and are also deeply immutable—then there is no need to repeatedly re-calculatehashCode
andtoString
results after their first call.I like the feature but have a few concerns before moving forward:
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?
The text was updated successfully, but these errors were encountered: