Steve,
I have made changes based on the github comments and your comments below
(
https://github.com/johnaoahra80/hibernate-orm/commits/HHH-9701).
Do you want me to squash the commits down to one? Not sure how this
would effect the comments you have already made on GH
Thanks
John
On 06/04/15 04:51, Steve Ebersole wrote:
On Thu, Apr 2, 2015 at 4:45 AM, John O'Hara <johara(a)redhat.com
<mailto:johara@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?
--
John O'Hara
johara(a)redhat.com
JBoss, by Red Hat
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor,
Berkshire, SI4 1TE, United Kingdom.
Registered in UK and Wales under Company Registration No. 3798903 Directors: Michael
Cunningham (USA), Charlie Peters (USA), Matt Parsons (USA) and Michael O'Neill
(Ireland).