[hibernate-dev] HHH-10162 Inheritance and L2 cache
Gail Badner
gbadner at redhat.com
Wed Mar 15 18:21:31 EDT 2017
Hi Christian,
More comments below...
On Fri, Mar 3, 2017 at 4:38 PM, Christian Beikov <christian.beikov at gmail.com
> wrote:
> Thanks for the comments, I have updated the PR.
>
> I think that it is important to have a complete fix and I am already
> working on that, but this issue was specifically about the L2 cache, so I
> wanted to keep that stuff separate.
>
> Here are some other polymorphism related issues:
>
> - https://hibernate.atlassian.net/browse/HHH-9845
> - https://hibernate.atlassian.net/browse/HHH-11280
> - https://hibernate.atlassian.net/browse/HHH-4742
> - https://hibernate.atlassian.net/browse/HHH-2927
>
> Thanks for looking into this. It is definitely and area that can/should be
improved.
> The fix provided by Josh Landin for HHH-11280 would break reference
> equality and I think that is something we all want to keep. Which brings us
> to the actual problems.
>
I agree. We definitely do not want to break reference equality.
>
> 1. Whenever we encounter an association that is of a polymorphic entity
> type, we should actually internally, regardless of what the user
> configures, always fetch that association with subtypes. Otherwise we could
> run into that same problem again, that we "pollute" the L1 cache with a
> proxy of the wrong type. Since we want to keep reference equality, we can't
> replace that instance.
>
IIUC, this is only a problem if the association is with a polymorphic
entity Class that has a subclass. Since a fix would probably affect
performance, I think it would be best to only deal with this particular
case. Maybe you are saying the same thing; I just wanted to clarify.
What you are suggesting is to eagerly fetch the association. IIUC, a
workaround is to map such associations as eager. That way a proxy would not
be created, avoiding the problem altogether.
IMO, if the entity is mapped as lazy, it really should be lazily-loaded.
That said, I think that it would be worthwhile for Hibernate to determine
the concrete subclass for the associated entity and create the appropriate
proxy. I can think of some alternatives, but I think they would all affect
performance. Maybe there should be a property for indicating the strategy
to use. Some strategies that come to mind are:
1) by default, create an uninitialized proxy using the type indicated in
the mapping (as is done currently);
2) try loading from second-level cache (as done in your fix for HHH-10162);
if the entity is not found in the cache, then execute a query to determine
the associated entity's concrete subclass;
3) add a join to the original query to determine the associated entity's
concrete subclass, and create the appropriate uninitialized proxy.
You mentioned that you are working on something. Is tihs along the lines of
what you are thinking? Did you have other ideas?
2. When using bytecode enhancement, the loading of *ToOne instances could
> be deferred to the first access.
>
Yes, this is another workaround.
>
> 3. Actually it would also be possible if property access is used without
> bytecode enhancement, but that would require to use a special proxy for
> every object that might contain a non-initialized polymorphic association.
> The speciality about it is that it loads the association on first access.
>
>
I'm not sure I'm following this.
> 4. Another thing that we should consider is "merging" of fetched state
> into existing instances of the L1 cache. Imagine a user loads an entity X
> and some of it's associations are lazy loaded. Then the user sets of a
> query that would fetch that association. Currently the associations will be
> left untouched because we skip any further processing as soon as we
> encounter the instance for X in the L1 cache. What we should actually do
> is, merge any fetched state into the instance if possible.
>
+1; I think this is a good improvement.
>
> Since 1. will probably affect performance, it might only make sense to do
> it along with 2. or 3.
>
> Regardless of these improvements, I think we should apply the L2 cache fix
> and handle the rest later.
>
> What do you say?
>
I think it would be better to come up with a hibernate property and
strategies, and allow an application to opt into a strategy, rather than
making a change that can affect performance by default.
>
> Mit freundlichen Grüßen,
> ------------------------------
> *Christian Beikov*
> Am 04.03.2017 um 00:42 schrieb Gail Badner:
>
> Hi Christian,
>
> I've added some comments to the PR.
>
> As I mentioned there, this appears to be a partial fix. If the application
> really requires the entity to implement the "correct" class, then this will
> still break for them when the entity is not in the cache.
>
> I'm not sure if a partial fix really suffices. IIUC, enhancement would
> work.
>
> What to others think about this?
>
> Regards,
> Gail
>
> On Thu, Mar 2, 2017 at 11:59 AM, Christian Beikov <
> christian.beikov at gmail.com> wrote:
>
>> I can, just wanted to hear some general opinions before creating the PR.
>> Will do that tomorrow.
>>
>>
>> Mit freundlichen Grüßen,
>> ------------------------------------------------------------------------
>> *Christian Beikov*
>> Am 02.03.2017 um 20:57 schrieb Gail Badner:
>> > Hi Christian,
>> >
>> > You mentioned that you reproduced the issue, but I don't see a test
>> > case. Can you push your test case to a PR?
>> >
>> > Thanks,
>> > Gail
>> >
>> > On Thu, Mar 2, 2017 at 10:15 AM, Steve Ebersole <steve at hibernate.org
>> > <mailto:steve at hibernate.org>> wrote:
>> >
>> > BTW, regarding the Hip Chat "Dev" room, I'd personally not rely on
>> > replies
>> > from there. E.g. I never saw your question there, and others
>> > likely did
>> > not either. FWIW we had all agreed on a block of time when we
>> > would try to
>> > be available in that room specifically to discuss such things.
>> > IIRC that
>> > was roughly between 10 and 12 my time (central US).
>> >
>> > On Thu, Mar 2, 2017 at 12:08 PM Christian Beikov
>> > <christian.beikov at gmail.com <mailto:christian.beikov at gmail.com>>
>> > wrote:
>> >
>> > > Hey guys,
>> > >
>> > > Steve said I should start a discussion about the possible
>> > solution for
>> > > HHH-10162 <https://hibernate.atlassian.net/browse/HHH-10162
>> > <https://hibernate.atlassian.net/browse/HHH-10162>> so here we
>> > > go.
>> > >
>> > > While debugging the issue, I found out that the proxy is created
>> at
>> > > DefaultLoadEventListener.createProxyIfNecessary() where it IMO
>> > should
>> > > consult the 2L cache first by calling existing =
>> > > loadFromSecondLevelCache( event, persister, keyToLoad );. The
>> > fix looks
>> > > easy, but I am not sure of the implications. Obviously this will
>> > affect
>> > > performance a little since it has to consult the L2 cache now.
>> > >
>> > > I tried to start a discussion in the Dev room, but so far only
>> > Andrea,
>> > > Vlad and Chris have commented this. Has anyone a different idea
>> for
>> > > implementing this?
>> > > --
>> > >
>> > > Mit freundlichen Grüßen,
>> > >
>> > -----------------------------------------------------------
>> -------------
>> > > *Christian Beikov*
>> > > _______________________________________________
>> > > hibernate-dev mailing list
>> > > hibernate-dev at lists.jboss.org <mailto:hibernate-dev at lists.jb
>> oss.org>
>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> > <https://lists.jboss.org/mailman/listinfo/hibernate-dev>
>> > _______________________________________________
>> > hibernate-dev mailing list
>> > hibernate-dev at lists.jboss.org <mailto:hibernate-dev at lists.jboss.org
>> >
>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> > <https://lists.jboss.org/mailman/listinfo/hibernate-dev>
>> >
>> >
>>
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>
>
>
>
More information about the hibernate-dev
mailing list