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(a)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/d9f5dc76e05a111d37db...
- Created a LockModeException for invalid lock modes for an
ImmutableEntity:
https://github.com/johnaoahra80/hibernate-orm/commit/99e308ae267f9bfc3054...
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/310a2eb5b8988189806a...
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/df50d344441ed4cc8f3c...
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(a)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(a)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'Harajohara(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).
>
>
>
--
John O'Harajohara(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).