Grails uses overly eager hibernate proxy unwrapping

243 views
Skip to first unread message

Aaron

unread,
Apr 21, 2015, 12:50:19 PM4/21/15
to grails-de...@googlegroups.com
As we start to move more and more to asynchronous programming models, I'm finding that Grails is WAY too eager about wanting to unwrap proxies, to the point of making them almost unusable in many cases.

For instance, given this scenario where a book is loaded in a different session (imagine the books are loaded in a thread, for instance):

Book book = null
Book.withNewSession {
    book = Book.get(1L)
}

Calling:


or even just doing something like:

new Book(author: book.author).save()

Causes a lazy initialization exception, even though nothing but the "id" of the Author is ever needed for this. I mean, if you can't even access the id of a proxy, what good is it?

The problem is that HibernateUtils.handleLazyProxy() is being invoked when "book.author" is called and wants to unwrap the author to "avoid proxy hell" as the comment says.

This example would work just fine without "avoiding proxy hell" and actually prevents the use of concurrent reader threads for batch processing or just simple asynchronous retrieval of related objects for other operations.

Is there some good reason (other than proxy hell, whatever that means) why handleLazyProxy() is so aggressive? Is there some way to disable this on a case-by-case basis? 

We have a scenario where we are loading thousands or records in multiple concurrent threads and then want to pass those along to something that is inserting data in multiple threads. We have to eagerly fetch all of the associated objects even though they are just being assigned across such as "a.someProperty = b.someProperty" with no actually use of the proxy. Obviously this is killing performance. In some cases, even using the handy and undocumented "Id" for Hibernate is still causing issues.

-Aaron

Aaron

unread,
Apr 21, 2015, 3:31:20 PM4/21/15
to grails-de...@googlegroups.com
So the more I think about this, the more it seems like this really can't be useful except possibly in some rare cases. The OpenSessionInView strategy tends to eliminate the majority of lazy initialization hell for typical applications. To prove my point, I built a new version of the grails-datastore-gorm-hibernate-core with this line commented out in the AbstractGrailsDomainBinder:

handleLazyProxy(grailsProperty.getDomainClass(), grailsProperty);

Our application starts up and doesn't have any immediately obvious issues. I opened a console and confirmed my changes along with testing a few cases of using proxies directly in withCriteria and DetachedCriteria and they all worked. I thought maybe something like Book.findAll { author == book.author } would fail if book.author returned a proxy...it doesn't...works great. Same thing with withCriteria. The only place that got me was an asynchronous section of code that ended up putting objects in a Map. The hashCode() method tried to unwrap (as it should) and it failed. Simple fix to just eager fetch that object in the threads and I'm up and running.

Next, I ran a big billing/invoicing batch job that processes a lot of data in multiple threads (so it was already pretty much lazy-safe). No issues with lazy initialization and an amazing reduction in the number of queries. Again, this is due to the fact that we are "copying" a lot of to-one associations that don't need to be unwrapped....a.something = b.something where b.something is a proxy.

I compared the SQL statements in the debug log for each version and the modified version ran significantly less SQL (around 20,000 less statements).

I'd really like to understand what problem this is supposed to be solving because it definitely impacts the default performance of our application. I suppose it might work to add an additional mapping parameter, similar to lazy: true, which allows you to skip this behavior on a per-property basis for performance reasons.

Graeme Rocher

unread,
Apr 22, 2015, 3:09:04 AM4/22/15
to grails-de...@googlegroups.com
Can you not use 'book.authorId' instead?
> --
> You received this message because you are subscribed to the Google Groups
> "Grails Dev Discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to grails-dev-disc...@googlegroups.com.
> To post to this group, send email to grails-de...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/grails-dev-discuss/e4708c70-7d4a-4c63-abef-f5d647d2c28e%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.



--
Graeme Rocher

Aaron

unread,
Apr 22, 2015, 8:08:26 AM4/22/15
to grails-de...@googlegroups.com
No, because book.authorId does not work for assignment (it's a readonly property). A very common scenario (translated into Book-Author model):

def newBook = new Book()
newBook.author = oldDetachedBook.author
newBook.save()

In this case, the author is unwrapped for no reason. I suppose a workaround might be to do Author.load(oldDetachedBook.authorId), but that seems like a silly requirement.

There are other problems with using the authorId syntax as well. The first is that it completely breaks the object abstraction. The second is that it doesn't work in normal unit tests (yes, I know it works in HibernateUnitTest, but they are impractical for anyone with a large connected object graph). It also percolates down into queries and such. Now instead of writing:

Using the authorId syntax just feels wrong. We've also found scenarios where it also seems to want to unwrap the proxy (I think for certain id generators, haven't tracked it down).

We have a very large, complex application and removing this aggressive unwrap, so far, doesn't seem to have broken a single thing. I just don't see the problem it's solving.

Aaron

unread,
Apr 22, 2015, 3:32:31 PM4/22/15
to grails-de...@googlegroups.com
Looking at the history for HibernateUtils, Lari Hotari has been down this road before. On 5/3/13 he committed a change that commented out the handle lazy and returned proxies. A few revisions later, he added that code back for "compatibility reasons".

I'd like to at least propose a property or configuration value that allows us to control this behavior in 2.4.x and later. I can submit a pull request against 2.4.6 if there is willingness. 

I also think Grails 3.0 and later should remove this behavior and return true proxies for performance reasons. For applications like ours, it can be a huge gain with zero additional effort.

-Aaron

Lari Hotari

unread,
Apr 23, 2015, 8:56:30 AM4/23/15
to grails-de...@googlegroups.com
Yes I remember working on that. I reverted some of the changes after some test failures. It turned out that it would have been a breaking change at that point to remove the eager proxy initialization.

This is the change that removed eagerly initializing proxies:
https://github.com/grails/grails-data-mapping/commit/37f5da37

And this is where I reverted it:
https://github.com/grails/grails-data-mapping/commit/8ef8d2b0

The eager proxy initialization was originally introduced by this commit:
https://github.com/grails/grails-core/commit/58182b40

That commit references:
https://jira.grails.org/browse/GRAILS-4242
https://jira.grails.org/browse/GRAILS-4230

-Lari

Lari Hotari

unread,
Apr 23, 2015, 9:11:47 AM4/23/15
to grails-de...@googlegroups.com
further on, those jira issues reference
http://jira.codehaus.org/browse/GROOVY-3433
and
https://hibernate.atlassian.net/browse/HHH-3870

I believe I've handled that problem in later proxy handling / generation changes that I worked on in grails-data-mapping.
like the commit "Hold MetaClass of Javassist proxy in the MethodHandler" https://github.com/grails/grails-data-mapping/commit/37f5da37 and related commit
https://github.com/grails/grails-data-mapping/commit/c7d6544
I've also made other changes in that area.

history of changes to org.grails.datastore.mapping.proxy package:
https://github.com/grails/grails-data-mapping/commits/master/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/proxy

The main goal of some of the changes were to solve a performance problem that showed up in performance tests:
https://jira.grails.org/browse/GRAILS-11614
and as the part of that, for consistency https://jira.grails.org/browse/GRAILS-11792

I hope this helps when digging in to the background of changes. :)

Lari

Aaron

unread,
Apr 23, 2015, 9:48:05 AM4/23/15
to grails-de...@googlegroups.com
Thanks Lari, that's very helpful.

Given that you fixed the metaClass issues in 37f5da37 and GRAILS-11614 fixed the performance issues with the proxy factory, do you think it's worth returning to the non-eager version and returning the proxies again?

Like I said, we are running with a version that does just that and have found 0 problems so far in 2.4.4. Performance is much better in many scenarios, particularly some of the batch jobs that were fetching/copying associations. I've been playing around with the behavior for many different scenarios and it seems to behave exactly as I'd expect.

-Aaron

Lari Hotari

unread,
Apr 29, 2015, 2:51:38 PM4/29/15
to grails-de...@googlegroups.com

Yes I think it would make sense to return proxies. There might be some other areas where proxies are initiated before the view is rendered making the proxies useless. I think that was the case at the time I was working on the changes. That code might have been removed already.
Since it's now easy to setup Travis CI for your own fork, I'd first suggest to run the tests against your fork with Travis CI to see if any other tests break after making the change.

-Lari
Reply all
Reply to author
Forward
0 new messages