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

Fri Apr 10 10:26:41 EDT 2015

I have updated my current branch: 
https://github.com/johnaoahra80/hibernate-orm/tree/HHH-9701

  * Extracted the deserialization of EntityEntry in EntityEntryContext
    into a separate method:
    https://github.com/johnaoahra80/hibernate-orm/commit/d9f5dc76e05a111d37dbdac64c6aa1bd485a1ba9
  * Created a LockModeException for invalid lock modes for an
    ImmutableEntity:
    https://github.com/johnaoahra80/hibernate-orm/commit/99e308ae267f9bfc30547784087955d7dc1e73e0

Do we need a new Exception type here, or could we just use a 
HibernateException with a message about invalid lock type?

  * Added a check when EntityEntryContext.clear() or EntityEntry.
    removeEntityEntry() is called to see if the EntityEntry is
    ImmutableEntityEntry AND the Entity is referenced cached in the 2lc:
    https://github.com/johnaoahra80/hibernate-orm/commit/310a2eb5b8988189806a2e688090a7ea16ae5474

Not sure of the performance implications on this, I am planning a run on 
WF9 to test and regressions in our use case.

  * Extracted an AbstractEntityEntry superclass that MutableEntityEntry
    and ImmutableEntityEntry both inherit from:
    https://github.com/johnaoahra80/hibernate-orm/commit/df50d344441ed4cc8f3c3c8df90edc106dd52adb

John

On 07/04/15 20:20, Steve Ebersole wrote:
> TBH, I have no idea what happens to comments on a Pull Request when 
> you squash and force push.
>
> I'd just leave the multiple commits.  We can squash them later.
>
> On Tue, Apr 7, 2015 at 1:59 PM, John O'Hara <johara at redhat.com 
> <mailto:johara at redhat.com>> wrote:
>
>     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  <mailto: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).
>
>

-- 
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).