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

[Steve Ebersole]

Well the performance is exactly what I had in mind when I asked you about
the exact "trigger for using ImmutableEntityEntry" in your use-case.  But
you never replied to that.



On Fri, Apr 10, 2015 at 9:26 AM, John O'Hara <johara at redhat.com> wrote:

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