[hibernate-dev] HHH-9701 - Develop "immutable EntityEntry" impl

[John O'Hara]

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 at redhat.com 
> <mailto:johara at 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/ec9b1fa3b97131ff1e65a1cc30ff6e4f2d2c8a28#diff-b55e1e51b30abe8d3c280bb22aeb6a44R380
>
>
> 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/ec9b1fa3b97131ff1e65a1cc30ff6e4f2d2c8a28#diff-b55e1e51b30abe8d3c280bb22aeb6a44L281
>
>
> 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 at 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).