On Thu, Apr 2, 2015 at 4:45 AM, John O'Hara <johara(a)redhat.com> wrote:
Steve,
I have pushed a proposal for HHH-9701 to:
https://github.com/johnaoahra80/hibernate-orm/tree/HHH-9701
There are a couple of areas that I would appreciate feedback;
1) Serialization/Deserialization - EntityEntries implementations can be
serialized and each implementation provide their own serialization method.
I have modified the serialization of EntityEntry in EntityEntryContext to
write the Implementation class to the OutputStream so the correct class can
be used to deserialize the object stream. Is the exception handling
sufficient here, or do we need more robust handling of deserialization
exceptions? : see
https://github.com/johnaoahra80/hibernate-orm/commit/ec9b1fa3b97131ff1e65...
I added some comments to that section. Also, overall I would extract the
deserialization bit into a separate method (deserializeEntityEntry).
2) In our (perf team) use case, we want to be able to share the
ImmutableEntityEntry between sessions when they are referenced cached
in
the 2lc. I have modified EntityEntryContext to not null
managedEntity.$$_hibernate_setEntityEntry if the EntityEntry is an instance
of ImmutableEntityEntry. Do we need to add an extra checks here, to ensure
that the entity is Reference Cached? I am not sure how we would test that
case? : see
https://github.com/johnaoahra80/hibernate-orm/commit/ec9b1fa3b97131ff1e65...
It really depends on what y'all consider the trigger for using
ImmutableEntityEntry. When would the EntityPersister use the
EntityEntryFactory producing ImmutableEntityEntry instances?
a) when the entity is marked immutable?
b) when the entity is marked immutable *and* we need to cache it by
reference?
c) some other condition?
I agree that we should only not clear that reference when the entity is
enabled for cache-by-reference. How that plays into this depends on the
answer to the above question.
If (a), then I think that yes it makes sense to add a check to only clear
the ManagedEntity's EntityEntry reference if using cache-by-reference.
If (b), then the EntiytPersister is only using the EntityEntryFactory
producing ImmutableEntityEntry instances when both are true. So the fact
that an entry is an instance of ImmutableEntityEntry indicates that we need
to not clear it from the ManagedEntity.
3) Lock Mode: Steve you mentioned about not doing locking for
Immutable
entities. Where is the locking implemented? Would it be sufficient to
simply set the LockMode on the ImmutableEntityEntry to NONE/READ_ONLY when
setLockMode is called?
Locking is implemented in many places.
What I had in mind, in terms of implementation for EntityEntry, is somewhat
influenced by the choice between ignore versus exception in cases where
something is not supported. Basically I had thought to throw an exception
in ImmutableEntityEntry#setLockMode or to simply ignore the call
altogether. This is not a great solution.
It is hard for me to justify ignoring the lock request in all cases. What
does everyone else think?