[hibernate-dev] HHH-9701 - Develop "immutable EntityEntry" impl
Steve Ebersole
steve at hibernate.org
Fri Apr 17 14:55:08 EDT 2015
I commented on your pull request.
My main concern is how you determine "canClearEntityEntryReference". That
needs to change. See my comment.
On Mon, Apr 13, 2015 at 5:37 AM, John O'Hara <johara at redhat.com> wrote:
> I think there are two factors to consider, when the entity is marked as
> Immutable, and when the EntityEntry can be shared between sessions.
>
> Our use case concerns the ImmutableEntityEntry being cached and shared
> between sessions, so we need to check if the Entity is reference cached in
> the 2lc.
>
> Therefore I think the answer to the question below is;
>
>
> - I would consider the the trigger for using ImmutableEntityEntry when
> the Entity is marked as Immutable;
> - I would consider the trigger for not nulling the EntityEntry in the
> EntityEntryContext when the EntityEntry is an ImmutableEntityEntry and the
> Entity is reference cached in the 2lc
>
>
> There might be further optimizations for ImmutableEntityEntries that are
> not cached and shared between sessions that I am not aware of.
>
> It seems the most logical place to determine whether to create a
> MutableEntityEntry or an ImmutableEntityEntry is when a call is made to
> AbstractEntityPersister getEntityEntryFactory() and we can test to see if
> the Entity is Immutable.
>
> I was unsure how of the best method to determine if the Enity would be
> cached in the 2lc, so tested at the point EntityEntry is nulled in
> EntityEntryContext. removeEntityEntry() or EntityEntryContext.clear()
>
>
>
> On 10/04/15 17:46, Steve Ebersole wrote:
>
> To post my question again...
>
> 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?
>
> Because the answer to this ^^ dictates how we need to check whether we
> can clear this reference
>
>
> On Fri, Apr 10, 2015 at 11:45 AM, Steve Ebersole <steve at hibernate.org>
> wrote:
>
>> 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).
>>>
>>>
>>>
>>
>
>
> --
> 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).
>
>
>
More information about the hibernate-dev
mailing list