Const data class

voddan commented on Sep 27, 2016 •

Please redo the benchmarking using JMH.

It is very hard to reason about the correctness of your current benchmark: maybe you forgot a tiny detail which changes everything, maybe you didn't. Only a dev of level "hotspot jvm maintainer" could tell the difference ;)

voddan commented on Sep 27, 2016

Another alternative:

Change the algorithm for all data classes to check that:

• no hashCode implementation is provided
• all properties in the constructor are val
• all properties are of
  • basic types: Int, Double, ets,String`,
  • known immutable types from JDK: BigInteger, ets?

Then implements a hashing function with hashing

voddan commented on Sep 27, 2016

This proposal seems to be also applicable to toString. Why not include it in the proposal?

voddan commented on Sep 27, 2016

About the alternative to implement @CachedHashcode:

A compiler-implemented annotation has an ability to check its applicant in detail. For instance it can check that the class is data and all properties are immutable. That way its functionality is not less than one of a keyword.

IRus commented on Dec 10, 2016

How it will works with serialization? I see possible problems of transferring hashCode between JVMs.

@voddan:

• Concerning the benchmark: sorry, I don't know what JVMH is. Can you be more specific ?
• I've updated the proposal to also cache toString ;)
• Whether it's a keyword (such as const) or an annotation (such as @CachedHashcode) doesn't really matter. I've updated the proposal in that sense.
• I like the auto-detection feature. I've updated the proposal ;) I think a keyword or annotation should still exist, though, to allow the programmer to force himself to enforce the limitations.

@IRus:

You're right. All cached values should be transient. I've updated the proposal.

voddan commented on Dec 10, 2016

@SalomonBrys auto detection will be inconsistent with the rest of Kotlin because it silently adds a field to a class. That may be critical when you optimize for small footprint.

voddan commented on Dec 10, 2016

What is the reasoning for making the cache @volatile?

@voddan, You're right. I've moved the auto-detection paragraph to the "alternatives" section, and annotated it with your reserve.

The cached values should be volatile because multiple threads may access hashCode at the same time. If the hash code is not computed, this may lead to corruption.

I'll update the benchmark using JVMH next week, when I'll have access to my linux PC ;)

voddan commented on Dec 10, 2016 •

The cached values should be volatile because multiple threads may access hashCode at the same time. If the hash code is not computed, this may lead to corruption.

What corruption may that cause? The hash is an Int which has the same value even if computed on different threads. The worst case scenario is that 2 threads compute the hash simultaneously and override one another with the same value. IMO no synchronisation is needed.

This is important because volatile reads may have a significant overhead http://stackoverflow.com/a/12357342/3144601

IRus commented on Dec 10, 2016 •

It's corner case, but on my jvm for 4294967297 number hashCode is 0, so your code will compute it every time)
Please add this case to tests too.

const data class can be good investment in the future when value types will come in Java 10 (probably). At this moment, such class can be converted into correct value-based class as stated here