vladmihalcea / hypersistence-optimizer

Hypersistence Optimizer allows you to get the most out of JPA and Hibernate. By scanning your application configuration and mappings, Hypersistence Optimizer can tell you what changes you need to do to speed up your data access layer.
https://vladmihalcea.com/hypersistence-optimizer/
Apache License 2.0
315 stars 43 forks source link

Check if @LazyToOne(LazyToOneOption.NO_PROXY) is set for parent-side OneToOne associations when using enableLazyInitialization bytecode enhancement #120

Closed pblanchardie closed 3 years ago

pblanchardie commented 3 years ago

Hypersistence warns about EAGER associations that should be LAZY, but does not warn about LAZY associations that remain EAGER when bytecode enhancement is on with enableLazyInitialization. Is Hypersistence aware of bytecode enhancement? Does it make sense to warn about that kind of misuse and potential Hibernate bugs?

As a hopefully temporary legacy hold-over, it is currently required that all lazy singular associations (many-to-one and one-to-one) also include @LazyToOne(LazyToOneOption.NO_PROXY). The plan is to relax that requirement later. https://docs.jboss.org/hibernate/orm/5.4/userguide/html_single/Hibernate_User_Guide.html#BytecodeEnhancement-capabilities

Moreover, detailed @LazyGroups are needed to avoid loading all associations together.

vladmihalcea commented 3 years ago

@pblanchardie This is a great idea. I'll surely add support for this check as well.

pblanchardie commented 3 years ago

Great! As mentioned in https://hibernate.atlassian.net/browse/HHH-13134?focusedCommentId=107604, setting hibernate.bytecode.allow_enhancement_as_proxy to true is the simplest workaround I found to stick with a "standard" lazy behaviour of ToOne.

vladmihalcea commented 3 years ago

You're welcome. At Hypersistence, we listen to our customers. I'll test the behavior for each HIbernate version and trigger the event based on what configuration options are discovered. Stay tuned for more awesome checks.

vladmihalcea commented 3 years ago

@pblanchardie I'm now working on this issue.

Basically, then lazy loading bytecode enhancement is enabled and NOPROXY is set, this is what we'd get:

OneToOneParentSideEvent oneToOneMappedByEvent = getTriggeredEvent(OneToOneParentSideEvent.class);
assertEquals(Event.Priority.MINOR, oneToOneMappedByEvent.getPriority());

assertEquals(Post.class, oneToOneMappedByEvent.getEntityClass());
assertEquals("details", oneToOneMappedByEvent.getEntityAttribute());
assertTrue(oneToOneMappedByEvent.isEnhancedForLazyLoading());
assertTrue(oneToOneMappedByEvent.isLazyToOneNoProxy());

Otherwise, if the lazy loading bytecode enhancement is enabled and NOPROXY is not set, this is what we'd get instead:

OneToOneParentSideEvent oneToOneMappedByEvent = getTriggeredEvent(OneToOneParentSideEvent.class);
assertEquals(Event.Priority.CRITICAL, oneToOneMappedByEvent.getPriority());

assertEquals(Post.class, oneToOneMappedByEvent.getEntityClass());
assertEquals("details", oneToOneMappedByEvent.getEntityAttribute());
assertTrue(oneToOneMappedByEvent.isEnhancedForLazyLoading());
assertFalse(oneToOneMappedByEvent.isLazyToOneNoProxy());

Notice that the priority changes and there are two new properties added for the OneToOneParentSideEvent.

vladmihalcea commented 3 years ago

Or, another alternative would be to avoid triggering the OneToOneParentSideEvent if lazy loading bytecode enhancement is enabled and NO_PROXY is set.

This is fine even in the context of the #126 issue.

vladmihalcea commented 3 years ago

Fixed.

pblanchardie commented 3 years ago

Thanks! I've noticed that you renamed this issue with @OneToOne instead of ToOne, so I guess your changes only apply to @OneToOne - and I read the tests. May I ask why is @ManyToOne not covered?

Is hibernate.bytecode.allow_enhancement_as_proxy taken into account, as it's an alternative to @LazyToOne(LazyToOneOption.NO_PROXY)?

Happy New Year!

vladmihalcea commented 3 years ago

@pblanchardie I've only used this feature for @OneToOne. I don't see any reason why this would ever be needed for @ManyToOne since the FK value allows Hibernate to assign either null or a Proxy.

pblanchardie commented 3 years ago

Sure, it's a Hibernate issue and I don't know if it will be fixed in 6.0, but fetch=LAZY is ignored on @ManyToOne and @OneToOne when bytecode enhancement with lazy initialization is on, unless NO_PROXY is added. This issue makes lazy initialization quite dangerous, or difficult to deal with. A "critical" problem in my opinion. That's why I reacted to your blog post about the danger of lazy initialization (nice for lazy basic attributes, but tragic for the rest of the app as it fundamentally changes load plans). Even join fetching can't solve n+1 selects, see https://stackoverflow.com/a/65389870/1458562. And the only undocumented workaround is bytecode enhancement as proxy.

vladmihalcea commented 3 years ago

@pblanchardie Thanks for pointing it out. I created #126 for that as I need more investigations about it.

pblanchardie commented 3 years ago

You're welcome and yes, it's another issue and this one is perfect as a check that OneToOne does not produce N+1.