[WIP] Add immutable interface as a hint to skip defensive cloning #11479
Conversation
We perform defensive cloning in some cases such as: - receiving user supplied objects - providing hazelcast internal object to the user If the user makes a promise not to mutate the object or provide state which changes during cloning, we can skip cloning and gain performance. See: hazelcast#11410
| * It is important that the user follows the rules: | ||
| * <ul> | ||
| * <li>the object must not have any state which is changed by cloning the object</li> | ||
| * <li>the existing state must not be changed</li> |
There was a problem hiding this comment.
Is the first one not implied by the second one?
There was a problem hiding this comment.
The first one should cover the case where the user supplies an predicate, for example, where the result of the predicate depends on some state which is lost on cloning.
The second one should cover the case where we provide the user an internal hz object like giving him the OBJECT instance to the map store, for example, and we expect that he will play by the rules and not change it.
There was a problem hiding this comment.
I tried to make the javadoc as clear I could at the moment but I must admit the wording might not be ideal. I would definitely like to make it crystal clear that the user should "play by the rules". So I'm open to suggestions.
| * @param <T> the type of the object | ||
| * @return the object clone or the provided object if it implements {@link Immutable} | ||
| */ | ||
| private <T> T cloneIfNecessary(T object) { |
There was a problem hiding this comment.
Alternative name: cloneIfMutable() (it's at least easier to spell and describes well what it does)
| import com.hazelcast.spi.annotation.Beta; | ||
|
|
||
| /** | ||
| * Allows notifying Hazelcast code that the object implementing this interface is effectively immutable. |
There was a problem hiding this comment.
If we keep this text, it's optimizations (American English) and Hazelcast (uppercase).
Alternative:
/**
* Marks classes that are (effectively) immutable.
* <p>
* Instances of immutable classes either have no state (e.g. pure functions) or the state
* is not mutated after the object construction (e.g. all fields are final). This allows
* Hazelcast to apply performance optimizations such as avoiding cloning user supplied
* objects or cloning Hazelcast internal objects supplied to the user.
* <p>
* It is important that to follows these rules:
* <ul>
* <li>the object must not have any state which is changed by cloning the object</li>
* <li>the existing state must not be changed</li>
* </ul>
* <b>Warning:</b> If a class implements this interface but does not follow these rules,
* the results of the execution are undefined.
*/
|
@mmedenjak: I prefer to move this to 3.10. |
|
Closing for inactivity. Please reopen if necessary. |
We perform defensive cloning in some cases such as:
If the user makes a promise not to mutate the object or provide state
which changes during cloning, we can skip cloning and gain performance.
NOTE If we decide not to merge this now, I will just remove the cloning for the
readFromEventJournalmethod.See:
#11410