Domain class reloading optimizations
Locked
 
|
I dug around on JIRA and there are a ton of tickets regarding this, but I was hoping to get some dev insight (I apologize if this has been discussed a ton before or if this is more of a user-list question....it seemed to be about the internals of grails classloading so I chose the dev list).
Our project has a ton of domain classes (352 to be precise) and the active reloading for domain classes is essentially useless for us. It seems like anytime a domain class is changed, the entire layer is reinitialized. I can understand the complexity involved in trying to reload this layer but it seems like most of it is centered around the persistence side of it. Many times, when we are changing a domain class, it is just a change in business logic that resides in the class, not a change to the persistent attributes. Is it technically possible to have some kind of "lightweight" reloading for domain classes that would skip most of the time consuming stuff and just reload the class like it would a service or controller?
If running in that mode, a change to a persistent attribute would require a server restart, but right now almost every change requires a restart (for us). In writing plugins, I've been digging around in the grails source and made use of some of the reloading components like GrailsProjectWatcher, but I'm not sure exactly how it ties into the reloading of the GORM layer. If there's something that can be improved, I'm willing to spend time to try and improve it with some direction. Obviously, our project would benefit immensely from this.
-Aaron
|
|
> Is it technically possible to have some kind of "lightweight" reloading for
> domain classes that would skip most of the time consuming stuff and just > reload the class like it would a service or controller? Good question. Grails would need to track changes to persistent properties, associations, custom mappings and constraints. I'm afraid I don't know whether this is feasible or not. It probably is but would require a fair bit of work in Grails core. -- Peter Ledbrook Grails Advocate SpringSource - A Division of VMware --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
|
Obviously it would be nice if Grails could "know" whether to do the lightweight reload, however, I was thinking about just a flag that would tell it not to ever try and do the full monty. When running in this mode, the developer is basically saying "I want to quickly reload non-persistent changes while completely and knowingly ignoring changes to persistent attributes". Seems like in that mode, Grails could ignore some of the more time consuming stuff without having to actually keep track.
I'm guessing the bulk of the reload is happening in the HibernateGrailsPlugin via helpers like HibernatePluginSupport.
-Aaron On Mon, Jun 18, 2012 at 5:23 AM, Peter Ledbrook <[hidden email]> wrote:
|
|
|
> Obviously it would be nice if Grails could "know" whether to do the
> lightweight reload, however, I was thinking about just a flag that would > tell it not to ever try and do the full monty. When running in this mode, > the developer is basically saying "I want to quickly reload non-persistent > changes while completely and knowingly ignoring changes to persistent > attributes". Seems like in that mode, Grails could ignore some of the more > time consuming stuff without having to actually keep track. I see. It should be possible, but I don't know the code well enough to say definitively or not. Peter -- Peter Ledbrook Grails Advocate SpringSource - A Division of VMware --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Domain class reloading optimizations
|
A flag like that would be good. I was thinking that to properly do this you would just need to diff the two database models after a domain class change. Hibernate has classes for tables, columns, FKs, etc. If there was no model change, there's no persistence change and the update was due to a method change or something else that doesn't require dropping everything on the Hibernate side, just a domain class reload.
Burt
|
|
|
I've made one change that seems to help with the reload. In the HibernatePluginSupport.groovy, when the onChange is triggered for a domain class change, the HibernateGormEnhancer was running for all persistent entities. I'm sure this is b/c of the way they can be interconnected, but I was able to get a speed-up by just re-enhancing the entity that changed:
I took the code on line 509 that does getPersistentEntities() and replaced it with something like this: // If fast refresh is configured, only refresh the entity that changed. This could lead to problems since
// entities might have dependencies that are not refreshed. def fastRefresh = Boolean.TRUE == application.flatConfig.get('grails.gorm.fastRefresh') if( fastRefresh && source ) {
PersistentEntity entity = mappingContext.getPersistentEntity(source.name) enhanceEntity(entity) } else {
for (PersistentEntity entity in mappingContext.getPersistentEntities()) { enhanceEntity(entity) } } The source is passed from the onChange event triggered when the hibernate plugin needs to reload. Since we know which specific files have changed, it would make sense to just re-enhance those classes.
I think there is still a race condition and some locking needed to prevent the application from accessing the new classes before they are fully enhanced (in some cases, if I'm quick enough I can catch it before the methods are loaded. In those cases I usually get a missing .get() or missing .save() error). Changing the domain class again will reload and add the missing methods.
-Aaron On Mon, Jun 18, 2012 at 10:57 AM, burtbeckwith <[hidden email]> wrote: A flag like that would be good. I was thinking that to properly do this you |
Re: Domain class reloading optimizations
Administrator
|
Pull request / patches welcome
Cheers On Tue, Jun 19, 2012 at 6:20 AM, Aaron Long <[hidden email]> wrote: > I've made one change that seems to help with the reload. In the > HibernatePluginSupport.groovy, when the onChange is triggered for a domain > class change, the HibernateGormEnhancer was running for all persistent > entities. I'm sure this is b/c of the way they can be interconnected, but I > was able to get a speed-up by just re-enhancing the entity that changed: > > I took the code on line 509 that does getPersistentEntities() and replaced > it with something like this: > > // If fast refresh is configured, only refresh the entity that > changed. This could lead to problems since > // entities might have dependencies that are not refreshed. > def fastRefresh = Boolean.TRUE == > application.flatConfig.get('grails.gorm.fastRefresh') > if( fastRefresh && source ) { > PersistentEntity entity = > mappingContext.getPersistentEntity(source.name) > enhanceEntity(entity) > } > else { > for (PersistentEntity entity in > mappingContext.getPersistentEntities()) { > enhanceEntity(entity) > } > } > > The source is passed from the onChange event triggered when the hibernate > plugin needs to reload. Since we know which specific files have changed, it > would make sense to just re-enhance those classes. > > I think there is still a race condition and some locking needed to prevent > the application from accessing the new classes before they are fully > enhanced (in some cases, if I'm quick enough I can catch it before the > methods are loaded. In those cases I usually get a missing .get() or missing > .save() error). Changing the domain class again will reload and add the > missing methods. > > -Aaron > > > On Mon, Jun 18, 2012 at 10:57 AM, burtbeckwith <[hidden email]> > wrote: >> >> A flag like that would be good. I was thinking that to properly do this >> you >> would just need to diff the two database models after a domain class >> change. >> Hibernate has classes for tables, columns, FKs, etc. If there was no model >> change, there's no persistence change and the update was due to a method >> change or something else that doesn't require dropping everything on the >> Hibernate side, just a domain class reload. >> >> Burt >> >> >> pledbrook wrote >> > >> >> Obviously it would be nice if Grails could "know" whether to do the >> >> lightweight reload, however, I was thinking about just a flag that >> >> would >> >> tell it not to ever try and do the full monty. When running in this >> >> mode, >> >> the developer is basically saying "I want to quickly reload >> >> non-persistent >> >> changes while completely and knowingly ignoring changes to persistent >> >> attributes". Seems like in that mode, Grails could ignore some of the >> >> more >> >> time consuming stuff without having to actually keep track. >> > >> > I see. It should be possible, but I don't know the code well enough to >> > say definitively or not. >> > >> > Peter >> > >> > -- >> > Peter Ledbrook >> > Grails Advocate >> > SpringSource - A Division of VMware >> > >> >> >> -- >> View this message in context: >> http://grails.1312388.n4.nabble.com/Domain-class-reloading-optimizations-tp4630188p4630326.html >> Sent from the Grails - dev mailing list archive at Nabble.com. >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > -- Graeme Rocher Grails Project Lead SpringSource - A Division of VMware http://www.springsource.com --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
|
Graeme, Is the flag even necessary? Looking at the history of that class, the code to reload all entities was there from the original dynamic reloading work you did on 3/10/2011. Do we know if targeted reloading of the domain classes will cause issues or was this just a case of not wanting to prematurely optimize?
-Aaron On Tue, Jun 19, 2012 at 2:44 AM, Graeme Rocher <[hidden email]> wrote: Pull request / patches welcome |
Re: Domain class reloading optimizations
|
Administrator
|
2.1.x branch, the flag is probably not necessary
Cheers On Tue, Jun 19, 2012 at 2:55 PM, Aaron Long <[hidden email]> wrote: > Graeme, > Is the flag even necessary? Looking at the history of that class, the code > to reload all entities was there from the original dynamic reloading work > you did on 3/10/2011. Do we know if targeted reloading of the domain classes > will cause issues or was this just a case of not wanting to prematurely > optimize? > > Also, for a change like this, which branch makes the most sense to use for > the development and testing? I was using 2.0.x b/c it's most compatible with > our project, but I wouldn't expect to see it in that release. Is 2.1.x still > open for changes like this? > > -Aaron > > > On Tue, Jun 19, 2012 at 2:44 AM, Graeme Rocher <[hidden email]> wrote: >> >> Pull request / patches welcome >> >> Cheers >> >> On Tue, Jun 19, 2012 at 6:20 AM, Aaron Long <[hidden email]> wrote: >> > I've made one change that seems to help with the reload. In the >> > HibernatePluginSupport.groovy, when the onChange is triggered for a >> > domain >> > class change, the HibernateGormEnhancer was running for all persistent >> > entities. I'm sure this is b/c of the way they can be interconnected, >> > but I >> > was able to get a speed-up by just re-enhancing the entity that changed: >> > >> > I took the code on line 509 that does getPersistentEntities() and >> > replaced >> > it with something like this: >> > >> > // If fast refresh is configured, only refresh the entity that >> > changed. This could lead to problems since >> > // entities might have dependencies that are not refreshed. >> > def fastRefresh = Boolean.TRUE == >> > application.flatConfig.get('grails.gorm.fastRefresh') >> > if( fastRefresh && source ) { >> > PersistentEntity entity = >> > mappingContext.getPersistentEntity(source.name) >> > enhanceEntity(entity) >> > } >> > else { >> > for (PersistentEntity entity in >> > mappingContext.getPersistentEntities()) { >> > enhanceEntity(entity) >> > } >> > } >> > >> > The source is passed from the onChange event triggered when the >> > hibernate >> > plugin needs to reload. Since we know which specific files have changed, >> > it >> > would make sense to just re-enhance those classes. >> > >> > I think there is still a race condition and some locking needed to >> > prevent >> > the application from accessing the new classes before they are fully >> > enhanced (in some cases, if I'm quick enough I can catch it before the >> > methods are loaded. In those cases I usually get a missing .get() or >> > missing >> > .save() error). Changing the domain class again will reload and add the >> > missing methods. >> > >> > -Aaron >> > >> > >> > On Mon, Jun 18, 2012 at 10:57 AM, burtbeckwith <[hidden email]> >> > wrote: >> >> >> >> A flag like that would be good. I was thinking that to properly do this >> >> you >> >> would just need to diff the two database models after a domain class >> >> change. >> >> Hibernate has classes for tables, columns, FKs, etc. If there was no >> >> model >> >> change, there's no persistence change and the update was due to a >> >> method >> >> change or something else that doesn't require dropping everything on >> >> the >> >> Hibernate side, just a domain class reload. >> >> >> >> Burt >> >> >> >> >> >> pledbrook wrote >> >> > >> >> >> Obviously it would be nice if Grails could "know" whether to do the >> >> >> lightweight reload, however, I was thinking about just a flag that >> >> >> would >> >> >> tell it not to ever try and do the full monty. When running in this >> >> >> mode, >> >> >> the developer is basically saying "I want to quickly reload >> >> >> non-persistent >> >> >> changes while completely and knowingly ignoring changes to >> >> >> persistent >> >> >> attributes". Seems like in that mode, Grails could ignore some of >> >> >> the >> >> >> more >> >> >> time consuming stuff without having to actually keep track. >> >> > >> >> > I see. It should be possible, but I don't know the code well enough >> >> > to >> >> > say definitively or not. >> >> > >> >> > Peter >> >> > >> >> > -- >> >> > Peter Ledbrook >> >> > Grails Advocate >> >> > SpringSource - A Division of VMware >> >> > >> >> >> >> >> >> -- >> >> View this message in context: >> >> >> >> http://grails.1312388.n4.nabble.com/Domain-class-reloading-optimizations-tp4630188p4630326.html >> >> Sent from the Grails - dev mailing list archive at Nabble.com. >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe from this list, please visit: >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> > >> >> >> >> -- >> Graeme Rocher >> Grails Project Lead >> SpringSource - A Division of VMware >> http://www.springsource.com >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > -- Graeme Rocher Grails Project Lead SpringSource - A Division of VMware http://www.springsource.com --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
|
Ok, so the reloading race condition can be helped a ton by expanding the scope of the isReloadInProgress() in the GrailsProjectWatcher. The problem I was seeing is that the isReloading flag doesn't become true until the GrailsPluginManagerReloadPlugin starts firing the class change events. There is a small amount of time between the initial detection of changes by the DirectoryWatcher and the reload event by spring. During that time, you can start using domain objects (possibly other things) and then have them be reloaded but not enhanced yet. This leads to random missing methods, etc.
Without a proper callback mechanism from the plugin's onChange event back to the project watcher, I'm not sure you can be 100% perfect but I just changed the code to lock around the FileChangeListener.onChange() and onNew() methods as well as around the entire firePendingClassChangeEvents() method. Also, to allow multiple places to safely lock and unlock, I changed from a boolean to a simple lockCounter and added some synchronization.
I assume this should be a separate pull request and ticket from the domain reload stuff (I'm testing both in the same build since they are related for me)? -Aaron On Tue, Jun 19, 2012 at 9:01 AM, Graeme Rocher <[hidden email]> wrote: 2.1.x branch, the flag is probably not necessary |
Re: Domain class reloading optimizations
|
Administrator
|
Yup separate pull request. Thanks for looking into this
On Wed, Jun 20, 2012 at 12:45 AM, Aaron Long <[hidden email]> wrote: > Ok, so the reloading race condition can be helped a ton by expanding the > scope of the isReloadInProgress() in the GrailsProjectWatcher. The problem I > was seeing is that the isReloading flag doesn't become true until > the GrailsPluginManagerReloadPlugin starts firing the class change events. > There is a small amount of time between the initial detection of changes by > the DirectoryWatcher and the reload event by spring. During that time, you > can start using domain objects (possibly other things) and then have them be > reloaded but not enhanced yet. This leads to random missing methods, etc. > > Without a proper callback mechanism from the plugin's onChange event back to > the project watcher, I'm not sure you can be 100% perfect but I just changed > the code to lock around the FileChangeListener.onChange() and onNew() > methods as well as around the entire firePendingClassChangeEvents() method. > Also, to allow multiple places to safely lock and unlock, I changed from a > boolean to a simple lockCounter and added some synchronization. > > I assume this should be a separate pull request and ticket from the domain > reload stuff (I'm testing both in the same build since they are related for > me)? > > -Aaron > > On Tue, Jun 19, 2012 at 9:01 AM, Graeme Rocher <[hidden email]> wrote: >> >> 2.1.x branch, the flag is probably not necessary >> >> Cheers >> >> On Tue, Jun 19, 2012 at 2:55 PM, Aaron Long <[hidden email]> wrote: >> > Graeme, >> > Is the flag even necessary? Looking at the history of that class, the >> > code >> > to reload all entities was there from the original dynamic reloading >> > work >> > you did on 3/10/2011. Do we know if targeted reloading of the domain >> > classes >> > will cause issues or was this just a case of not wanting to prematurely >> > optimize? >> > >> > Also, for a change like this, which branch makes the most sense to use >> > for >> > the development and testing? I was using 2.0.x b/c it's most compatible >> > with >> > our project, but I wouldn't expect to see it in that release. Is 2.1.x >> > still >> > open for changes like this? >> > >> > -Aaron >> > >> > >> > On Tue, Jun 19, 2012 at 2:44 AM, Graeme Rocher <[hidden email]> >> > wrote: >> >> >> >> Pull request / patches welcome >> >> >> >> Cheers >> >> >> >> On Tue, Jun 19, 2012 at 6:20 AM, Aaron Long <[hidden email]> wrote: >> >> > I've made one change that seems to help with the reload. In the >> >> > HibernatePluginSupport.groovy, when the onChange is triggered for a >> >> > domain >> >> > class change, the HibernateGormEnhancer was running for all >> >> > persistent >> >> > entities. I'm sure this is b/c of the way they can be interconnected, >> >> > but I >> >> > was able to get a speed-up by just re-enhancing the entity that >> >> > changed: >> >> > >> >> > I took the code on line 509 that does getPersistentEntities() and >> >> > replaced >> >> > it with something like this: >> >> > >> >> > // If fast refresh is configured, only refresh the entity >> >> > that >> >> > changed. This could lead to problems since >> >> > // entities might have dependencies that are not refreshed. >> >> > def fastRefresh = Boolean.TRUE == >> >> > application.flatConfig.get('grails.gorm.fastRefresh') >> >> > if( fastRefresh && source ) { >> >> > PersistentEntity entity = >> >> > mappingContext.getPersistentEntity(source.name) >> >> > enhanceEntity(entity) >> >> > } >> >> > else { >> >> > for (PersistentEntity entity in >> >> > mappingContext.getPersistentEntities()) { >> >> > enhanceEntity(entity) >> >> > } >> >> > } >> >> > >> >> > The source is passed from the onChange event triggered when the >> >> > hibernate >> >> > plugin needs to reload. Since we know which specific files have >> >> > changed, >> >> > it >> >> > would make sense to just re-enhance those classes. >> >> > >> >> > I think there is still a race condition and some locking needed to >> >> > prevent >> >> > the application from accessing the new classes before they are fully >> >> > enhanced (in some cases, if I'm quick enough I can catch it before >> >> > the >> >> > methods are loaded. In those cases I usually get a missing .get() or >> >> > missing >> >> > .save() error). Changing the domain class again will reload and add >> >> > the >> >> > missing methods. >> >> > >> >> > -Aaron >> >> > >> >> > >> >> > On Mon, Jun 18, 2012 at 10:57 AM, burtbeckwith >> >> > <[hidden email]> >> >> > wrote: >> >> >> >> >> >> A flag like that would be good. I was thinking that to properly do >> >> >> this >> >> >> you >> >> >> would just need to diff the two database models after a domain class >> >> >> change. >> >> >> Hibernate has classes for tables, columns, FKs, etc. If there was no >> >> >> model >> >> >> change, there's no persistence change and the update was due to a >> >> >> method >> >> >> change or something else that doesn't require dropping everything on >> >> >> the >> >> >> Hibernate side, just a domain class reload. >> >> >> >> >> >> Burt >> >> >> >> >> >> >> >> >> pledbrook wrote >> >> >> > >> >> >> >> Obviously it would be nice if Grails could "know" whether to do >> >> >> >> the >> >> >> >> lightweight reload, however, I was thinking about just a flag >> >> >> >> that >> >> >> >> would >> >> >> >> tell it not to ever try and do the full monty. When running in >> >> >> >> this >> >> >> >> mode, >> >> >> >> the developer is basically saying "I want to quickly reload >> >> >> >> non-persistent >> >> >> >> changes while completely and knowingly ignoring changes to >> >> >> >> persistent >> >> >> >> attributes". Seems like in that mode, Grails could ignore some of >> >> >> >> the >> >> >> >> more >> >> >> >> time consuming stuff without having to actually keep track. >> >> >> > >> >> >> > I see. It should be possible, but I don't know the code well >> >> >> > enough >> >> >> > to >> >> >> > say definitively or not. >> >> >> > >> >> >> > Peter >> >> >> > >> >> >> > -- >> >> >> > Peter Ledbrook >> >> >> > Grails Advocate >> >> >> > SpringSource - A Division of VMware >> >> >> > >> >> >> >> >> >> >> >> >> -- >> >> >> View this message in context: >> >> >> >> >> >> >> >> >> http://grails.1312388.n4.nabble.com/Domain-class-reloading-optimizations-tp4630188p4630326.html >> >> >> Sent from the Grails - dev mailing list archive at Nabble.com. >> >> >> >> >> >> >> >> >> --------------------------------------------------------------------- >> >> >> To unsubscribe from this list, please visit: >> >> >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> -- >> >> Graeme Rocher >> >> Grails Project Lead >> >> SpringSource - A Division of VMware >> >> http://www.springsource.com >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe from this list, please visit: >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> > >> >> >> >> -- >> Graeme Rocher >> Grails Project Lead >> SpringSource - A Division of VMware >> http://www.springsource.com >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > -- Graeme Rocher Grails Project Lead SpringSource - A Division of VMware http://www.springsource.com --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
|
I was having trouble with the addTo* methods not being added to the domain classes after reload. After digging around, I found something that seems suspicious.
In GrailsDomainClassPersistentEntity, there's a list of associations which is returned by getAssociations(). It seems like that's only used in the GormEnhancer to iterate and add the addTo* methods. Problem is, that list always seems to be empty. Looking at the GrailsDomainClassPersistentEntity.initialize() method, I don't see anywhere the associations list is populated. I added a snippet at the end of initialize like:
if( persistentProperty instanceof Association ) { associations.add((Association)persistentProperty); } That does cause the associations to be populated so maybe I'm not crazy. The list isn't immutable so I guess someone could also call getAssociations() and add to it.
-Aaron
On Wed, Jun 20, 2012 at 2:42 AM, Graeme Rocher <[hidden email]> wrote: Yup separate pull request. Thanks for looking into this |
Re: Domain class reloading optimizations
|
Administrator
|
On Thu, Jun 21, 2012 at 12:57 AM, Aaron Long <[hidden email]> wrote:
> I was having trouble with the addTo* methods not being added to the domain > classes after reload. After digging around, I found something that seems > suspicious. > > In GrailsDomainClassPersistentEntity, there's a list of associations which > is returned by getAssociations(). It seems like that's only used in the > GormEnhancer to iterate and add the addTo* methods. Problem is, that list > always seems to be empty. Looking at the > GrailsDomainClassPersistentEntity.initialize() method, I don't see anywhere > the associations list is populated. I added a snippet at the end of > initialize like: > > if( persistentProperty instanceof Association ) { > associations.add((Association)persistentProperty); > } > > That does cause the associations to be populated so maybe I'm not crazy. The > list isn't immutable so I guess someone could also call getAssociations() > and add to it. Hmm yeah this seems wrong, definitely worth fixing Cheers > > -Aaron > > On Wed, Jun 20, 2012 at 2:42 AM, Graeme Rocher <[hidden email]> wrote: >> >> Yup separate pull request. Thanks for looking into this >> >> >> On Wed, Jun 20, 2012 at 12:45 AM, Aaron Long <[hidden email]> wrote: >> > Ok, so the reloading race condition can be helped a ton by expanding the >> > scope of the isReloadInProgress() in the GrailsProjectWatcher. The >> > problem I >> > was seeing is that the isReloading flag doesn't become true until >> > the GrailsPluginManagerReloadPlugin starts firing the class change >> > events. >> > There is a small amount of time between the initial detection of changes >> > by >> > the DirectoryWatcher and the reload event by spring. During that time, >> > you >> > can start using domain objects (possibly other things) and then have >> > them be >> > reloaded but not enhanced yet. This leads to random missing methods, >> > etc. >> > >> > Without a proper callback mechanism from the plugin's onChange event >> > back to >> > the project watcher, I'm not sure you can be 100% perfect but I just >> > changed >> > the code to lock around the FileChangeListener.onChange() and onNew() >> > methods as well as around the entire firePendingClassChangeEvents() >> > method. >> > Also, to allow multiple places to safely lock and unlock, I changed from >> > a >> > boolean to a simple lockCounter and added some synchronization. >> > >> > I assume this should be a separate pull request and ticket from the >> > domain >> > reload stuff (I'm testing both in the same build since they are related >> > for >> > me)? >> > >> > -Aaron >> > >> > On Tue, Jun 19, 2012 at 9:01 AM, Graeme Rocher <[hidden email]> >> > wrote: >> >> >> >> 2.1.x branch, the flag is probably not necessary >> >> >> >> Cheers >> >> >> >> On Tue, Jun 19, 2012 at 2:55 PM, Aaron Long <[hidden email]> wrote: >> >> > Graeme, >> >> > Is the flag even necessary? Looking at the history of that class, the >> >> > code >> >> > to reload all entities was there from the original dynamic reloading >> >> > work >> >> > you did on 3/10/2011. Do we know if targeted reloading of the domain >> >> > classes >> >> > will cause issues or was this just a case of not wanting to >> >> > prematurely >> >> > optimize? >> >> > >> >> > Also, for a change like this, which branch makes the most sense to >> >> > use >> >> > for >> >> > the development and testing? I was using 2.0.x b/c it's most >> >> > compatible >> >> > with >> >> > our project, but I wouldn't expect to see it in that release. Is >> >> > 2.1.x >> >> > still >> >> > open for changes like this? >> >> > >> >> > -Aaron >> >> > >> >> > >> >> > On Tue, Jun 19, 2012 at 2:44 AM, Graeme Rocher <[hidden email]> >> >> > wrote: >> >> >> >> >> >> Pull request / patches welcome >> >> >> >> >> >> Cheers >> >> >> >> >> >> On Tue, Jun 19, 2012 at 6:20 AM, Aaron Long <[hidden email]> >> >> >> wrote: >> >> >> > I've made one change that seems to help with the reload. In the >> >> >> > HibernatePluginSupport.groovy, when the onChange is triggered for >> >> >> > a >> >> >> > domain >> >> >> > class change, the HibernateGormEnhancer was running for all >> >> >> > persistent >> >> >> > entities. I'm sure this is b/c of the way they can be >> >> >> > interconnected, >> >> >> > but I >> >> >> > was able to get a speed-up by just re-enhancing the entity that >> >> >> > changed: >> >> >> > >> >> >> > I took the code on line 509 that does getPersistentEntities() and >> >> >> > replaced >> >> >> > it with something like this: >> >> >> > >> >> >> > // If fast refresh is configured, only refresh the entity >> >> >> > that >> >> >> > changed. This could lead to problems since >> >> >> > // entities might have dependencies that are not >> >> >> > refreshed. >> >> >> > def fastRefresh = Boolean.TRUE == >> >> >> > application.flatConfig.get('grails.gorm.fastRefresh') >> >> >> > if( fastRefresh && source ) { >> >> >> > PersistentEntity entity = >> >> >> > mappingContext.getPersistentEntity(source.name) >> >> >> > enhanceEntity(entity) >> >> >> > } >> >> >> > else { >> >> >> > for (PersistentEntity entity in >> >> >> > mappingContext.getPersistentEntities()) { >> >> >> > enhanceEntity(entity) >> >> >> > } >> >> >> > } >> >> >> > >> >> >> > The source is passed from the onChange event triggered when the >> >> >> > hibernate >> >> >> > plugin needs to reload. Since we know which specific files have >> >> >> > changed, >> >> >> > it >> >> >> > would make sense to just re-enhance those classes. >> >> >> > >> >> >> > I think there is still a race condition and some locking needed to >> >> >> > prevent >> >> >> > the application from accessing the new classes before they are >> >> >> > fully >> >> >> > enhanced (in some cases, if I'm quick enough I can catch it before >> >> >> > the >> >> >> > methods are loaded. In those cases I usually get a missing .get() >> >> >> > or >> >> >> > missing >> >> >> > .save() error). Changing the domain class again will reload and >> >> >> > add >> >> >> > the >> >> >> > missing methods. >> >> >> > >> >> >> > -Aaron >> >> >> > >> >> >> > >> >> >> > On Mon, Jun 18, 2012 at 10:57 AM, burtbeckwith >> >> >> > <[hidden email]> >> >> >> > wrote: >> >> >> >> >> >> >> >> A flag like that would be good. I was thinking that to properly >> >> >> >> do >> >> >> >> this >> >> >> >> you >> >> >> >> would just need to diff the two database models after a domain >> >> >> >> class >> >> >> >> change. >> >> >> >> Hibernate has classes for tables, columns, FKs, etc. If there was >> >> >> >> no >> >> >> >> model >> >> >> >> change, there's no persistence change and the update was due to a >> >> >> >> method >> >> >> >> change or something else that doesn't require dropping everything >> >> >> >> on >> >> >> >> the >> >> >> >> Hibernate side, just a domain class reload. >> >> >> >> >> >> >> >> Burt >> >> >> >> >> >> >> >> >> >> >> >> pledbrook wrote >> >> >> >> > >> >> >> >> >> Obviously it would be nice if Grails could "know" whether to >> >> >> >> >> do >> >> >> >> >> the >> >> >> >> >> lightweight reload, however, I was thinking about just a flag >> >> >> >> >> that >> >> >> >> >> would >> >> >> >> >> tell it not to ever try and do the full monty. When running in >> >> >> >> >> this >> >> >> >> >> mode, >> >> >> >> >> the developer is basically saying "I want to quickly reload >> >> >> >> >> non-persistent >> >> >> >> >> changes while completely and knowingly ignoring changes to >> >> >> >> >> persistent >> >> >> >> >> attributes". Seems like in that mode, Grails could ignore some >> >> >> >> >> of >> >> >> >> >> the >> >> >> >> >> more >> >> >> >> >> time consuming stuff without having to actually keep track. >> >> >> >> > >> >> >> >> > I see. It should be possible, but I don't know the code well >> >> >> >> > enough >> >> >> >> > to >> >> >> >> > say definitively or not. >> >> >> >> > >> >> >> >> > Peter >> >> >> >> > >> >> >> >> > -- >> >> >> >> > Peter Ledbrook >> >> >> >> > Grails Advocate >> >> >> >> > SpringSource - A Division of VMware >> >> >> >> > >> >> >> >> >> >> >> >> >> >> >> >> -- >> >> >> >> View this message in context: >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> http://grails.1312388.n4.nabble.com/Domain-class-reloading-optimizations-tp4630188p4630326.html >> >> >> >> Sent from the Grails - dev mailing list archive at Nabble.com. >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> --------------------------------------------------------------------- >> >> >> >> To unsubscribe from this list, please visit: >> >> >> >> >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> >> >> >> >> -- >> >> >> Graeme Rocher >> >> >> Grails Project Lead >> >> >> SpringSource - A Division of VMware >> >> >> http://www.springsource.com >> >> >> >> >> >> >> >> >> --------------------------------------------------------------------- >> >> >> To unsubscribe from this list, please visit: >> >> >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> -- >> >> Graeme Rocher >> >> Grails Project Lead >> >> SpringSource - A Division of VMware >> >> http://www.springsource.com >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe from this list, please visit: >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> > >> >> >> >> -- >> Graeme Rocher >> Grails Project Lead >> SpringSource - A Division of VMware >> http://www.springsource.com >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > -- Graeme Rocher Grails Project Lead SpringSource - A Division of VMware http://www.springsource.com --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
|
Well, it's weird. The GormEnhancer is the only user of the associations property on that class and it may not be a problem because the DomainClassGrailsPlugin also adds those methods.
I'm certainly confused why both the DomainClass plugin and the Hibernate plugin are adding the same methods. I would assume the domain class plugin would add generic, database agnostic stuff while the hibernate plugin would enhance the rest. The implementations are similar though not exact copies. I assume in the process of splitting dependence on hibernate, some of the responsibility is not clearly separated (just guessing). They both also re-register the Validator bean (although the domain class plugin is the only one that re-registers the other beans).
BTW, I think I've started into a deep dark hole here :) On Thu, Jun 21, 2012 at 3:11 AM, Graeme Rocher <[hidden email]> wrote:
|
Re: Domain class reloading optimizations
|
Administrator
|
On Thu, Jun 21, 2012 at 5:01 PM, Aaron Long <[hidden email]> wrote:
> Well, it's weird. The GormEnhancer is the only user of the associations > property on that class and it may not be a problem because the > DomainClassGrailsPlugin also adds those methods. > > I'm certainly confused why both the DomainClass plugin and the Hibernate > plugin are adding the same methods. I would assume the domain class plugin > would add generic, database agnostic stuff while the hibernate plugin would > enhance the rest. The implementations are similar though not exact copies. I > assume in the process of splitting dependence on hibernate, some of the > responsibility is not clearly separated (just guessing). They both also > re-register the Validator bean (although the domain class plugin is the only > one that re-registers the other beans). Yes this is some separation that probably is not clean. They both do register validator beans, but the hibernate plugin overrides the domain class one (to support unique constraint / persistent constraints etc.) Cheers > > BTW, I think I've started into a deep dark hole here :) > > On Thu, Jun 21, 2012 at 3:11 AM, Graeme Rocher <[hidden email]> wrote: >> >> On Thu, Jun 21, 2012 at 12:57 AM, Aaron Long <[hidden email]> wrote: >> > I was having trouble with the addTo* methods not being added to the >> > domain >> > classes after reload. After digging around, I found something that seems >> > suspicious. >> > >> > In GrailsDomainClassPersistentEntity, there's a list of associations >> > which >> > is returned by getAssociations(). It seems like that's only used in the >> > GormEnhancer to iterate and add the addTo* methods. Problem is, that >> > list >> > always seems to be empty. Looking at the >> > GrailsDomainClassPersistentEntity.initialize() method, I don't see >> > anywhere >> > the associations list is populated. I added a snippet at the end of >> > initialize like: >> > >> > if( persistentProperty instanceof Association ) { >> > associations.add((Association)persistentProperty); >> > } >> > >> > That does cause the associations to be populated so maybe I'm not crazy. >> > The >> > list isn't immutable so I guess someone could also call >> > getAssociations() >> > and add to it. >> >> Hmm yeah this seems wrong, definitely worth fixing >> >> Cheers >> >> > >> > -Aaron >> > >> > On Wed, Jun 20, 2012 at 2:42 AM, Graeme Rocher <[hidden email]> >> > wrote: >> >> >> >> Yup separate pull request. Thanks for looking into this >> >> >> >> >> >> On Wed, Jun 20, 2012 at 12:45 AM, Aaron Long <[hidden email]> wrote: >> >> > Ok, so the reloading race condition can be helped a ton by expanding >> >> > the >> >> > scope of the isReloadInProgress() in the GrailsProjectWatcher. The >> >> > problem I >> >> > was seeing is that the isReloading flag doesn't become true until >> >> > the GrailsPluginManagerReloadPlugin starts firing the class change >> >> > events. >> >> > There is a small amount of time between the initial detection of >> >> > changes >> >> > by >> >> > the DirectoryWatcher and the reload event by spring. During that >> >> > time, >> >> > you >> >> > can start using domain objects (possibly other things) and then have >> >> > them be >> >> > reloaded but not enhanced yet. This leads to random missing methods, >> >> > etc. >> >> > >> >> > Without a proper callback mechanism from the plugin's onChange event >> >> > back to >> >> > the project watcher, I'm not sure you can be 100% perfect but I just >> >> > changed >> >> > the code to lock around the FileChangeListener.onChange() and onNew() >> >> > methods as well as around the entire firePendingClassChangeEvents() >> >> > method. >> >> > Also, to allow multiple places to safely lock and unlock, I changed >> >> > from >> >> > a >> >> > boolean to a simple lockCounter and added some synchronization. >> >> > >> >> > I assume this should be a separate pull request and ticket from the >> >> > domain >> >> > reload stuff (I'm testing both in the same build since they are >> >> > related >> >> > for >> >> > me)? >> >> > >> >> > -Aaron >> >> > >> >> > On Tue, Jun 19, 2012 at 9:01 AM, Graeme Rocher <[hidden email]> >> >> > wrote: >> >> >> >> >> >> 2.1.x branch, the flag is probably not necessary >> >> >> >> >> >> Cheers >> >> >> >> >> >> On Tue, Jun 19, 2012 at 2:55 PM, Aaron Long <[hidden email]> >> >> >> wrote: >> >> >> > Graeme, >> >> >> > Is the flag even necessary? Looking at the history of that class, >> >> >> > the >> >> >> > code >> >> >> > to reload all entities was there from the original dynamic >> >> >> > reloading >> >> >> > work >> >> >> > you did on 3/10/2011. Do we know if targeted reloading of the >> >> >> > domain >> >> >> > classes >> >> >> > will cause issues or was this just a case of not wanting to >> >> >> > prematurely >> >> >> > optimize? >> >> >> > >> >> >> > Also, for a change like this, which branch makes the most sense to >> >> >> > use >> >> >> > for >> >> >> > the development and testing? I was using 2.0.x b/c it's most >> >> >> > compatible >> >> >> > with >> >> >> > our project, but I wouldn't expect to see it in that release. Is >> >> >> > 2.1.x >> >> >> > still >> >> >> > open for changes like this? >> >> >> > >> >> >> > -Aaron >> >> >> > >> >> >> > >> >> >> > On Tue, Jun 19, 2012 at 2:44 AM, Graeme Rocher >> >> >> > <[hidden email]> >> >> >> > wrote: >> >> >> >> >> >> >> >> Pull request / patches welcome >> >> >> >> >> >> >> >> Cheers >> >> >> >> >> >> >> >> On Tue, Jun 19, 2012 at 6:20 AM, Aaron Long <[hidden email]> >> >> >> >> wrote: >> >> >> >> > I've made one change that seems to help with the reload. In the >> >> >> >> > HibernatePluginSupport.groovy, when the onChange is triggered >> >> >> >> > for >> >> >> >> > a >> >> >> >> > domain >> >> >> >> > class change, the HibernateGormEnhancer was running for all >> >> >> >> > persistent >> >> >> >> > entities. I'm sure this is b/c of the way they can be >> >> >> >> > interconnected, >> >> >> >> > but I >> >> >> >> > was able to get a speed-up by just re-enhancing the entity that >> >> >> >> > changed: >> >> >> >> > >> >> >> >> > I took the code on line 509 that does getPersistentEntities() >> >> >> >> > and >> >> >> >> > replaced >> >> >> >> > it with something like this: >> >> >> >> > >> >> >> >> > // If fast refresh is configured, only refresh the >> >> >> >> > entity >> >> >> >> > that >> >> >> >> > changed. This could lead to problems since >> >> >> >> > // entities might have dependencies that are not >> >> >> >> > refreshed. >> >> >> >> > def fastRefresh = Boolean.TRUE == >> >> >> >> > application.flatConfig.get('grails.gorm.fastRefresh') >> >> >> >> > if( fastRefresh && source ) { >> >> >> >> > PersistentEntity entity = >> >> >> >> > mappingContext.getPersistentEntity(source.name) >> >> >> >> > enhanceEntity(entity) >> >> >> >> > } >> >> >> >> > else { >> >> >> >> > for (PersistentEntity entity in >> >> >> >> > mappingContext.getPersistentEntities()) { >> >> >> >> > enhanceEntity(entity) >> >> >> >> > } >> >> >> >> > } >> >> >> >> > >> >> >> >> > The source is passed from the onChange event triggered when the >> >> >> >> > hibernate >> >> >> >> > plugin needs to reload. Since we know which specific files have >> >> >> >> > changed, >> >> >> >> > it >> >> >> >> > would make sense to just re-enhance those classes. >> >> >> >> > >> >> >> >> > I think there is still a race condition and some locking needed >> >> >> >> > to >> >> >> >> > prevent >> >> >> >> > the application from accessing the new classes before they are >> >> >> >> > fully >> >> >> >> > enhanced (in some cases, if I'm quick enough I can catch it >> >> >> >> > before >> >> >> >> > the >> >> >> >> > methods are loaded. In those cases I usually get a missing >> >> >> >> > .get() >> >> >> >> > or >> >> >> >> > missing >> >> >> >> > .save() error). Changing the domain class again will reload and >> >> >> >> > add >> >> >> >> > the >> >> >> >> > missing methods. >> >> >> >> > >> >> >> >> > -Aaron >> >> >> >> > >> >> >> >> > >> >> >> >> > On Mon, Jun 18, 2012 at 10:57 AM, burtbeckwith >> >> >> >> > <[hidden email]> >> >> >> >> > wrote: >> >> >> >> >> >> >> >> >> >> A flag like that would be good. I was thinking that to >> >> >> >> >> properly >> >> >> >> >> do >> >> >> >> >> this >> >> >> >> >> you >> >> >> >> >> would just need to diff the two database models after a domain >> >> >> >> >> class >> >> >> >> >> change. >> >> >> >> >> Hibernate has classes for tables, columns, FKs, etc. If there >> >> >> >> >> was >> >> >> >> >> no >> >> >> >> >> model >> >> >> >> >> change, there's no persistence change and the update was due >> >> >> >> >> to a >> >> >> >> >> method >> >> >> >> >> change or something else that doesn't require dropping >> >> >> >> >> everything >> >> >> >> >> on >> >> >> >> >> the >> >> >> >> >> Hibernate side, just a domain class reload. >> >> >> >> >> >> >> >> >> >> Burt >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> pledbrook wrote >> >> >> >> >> > >> >> >> >> >> >> Obviously it would be nice if Grails could "know" whether >> >> >> >> >> >> to >> >> >> >> >> >> do >> >> >> >> >> >> the >> >> >> >> >> >> lightweight reload, however, I was thinking about just a >> >> >> >> >> >> flag >> >> >> >> >> >> that >> >> >> >> >> >> would >> >> >> >> >> >> tell it not to ever try and do the full monty. When running >> >> >> >> >> >> in >> >> >> >> >> >> this >> >> >> >> >> >> mode, >> >> >> >> >> >> the developer is basically saying "I want to quickly reload >> >> >> >> >> >> non-persistent >> >> >> >> >> >> changes while completely and knowingly ignoring changes to >> >> >> >> >> >> persistent >> >> >> >> >> >> attributes". Seems like in that mode, Grails could ignore >> >> >> >> >> >> some >> >> >> >> >> >> of >> >> >> >> >> >> the >> >> >> >> >> >> more >> >> >> >> >> >> time consuming stuff without having to actually keep track. >> >> >> >> >> > >> >> >> >> >> > I see. It should be possible, but I don't know the code well >> >> >> >> >> > enough >> >> >> >> >> > to >> >> >> >> >> > say definitively or not. >> >> >> >> >> > >> >> >> >> >> > Peter >> >> >> >> >> > >> >> >> >> >> > -- >> >> >> >> >> > Peter Ledbrook >> >> >> >> >> > Grails Advocate >> >> >> >> >> > SpringSource - A Division of VMware >> >> >> >> >> > >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> -- >> >> >> >> >> View this message in context: >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> http://grails.1312388.n4.nabble.com/Domain-class-reloading-optimizations-tp4630188p4630326.html >> >> >> >> >> Sent from the Grails - dev mailing list archive at Nabble.com. >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> --------------------------------------------------------------------- >> >> >> >> >> To unsubscribe from this list, please visit: >> >> >> >> >> >> >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> >> >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> -- >> >> >> >> Graeme Rocher >> >> >> >> Grails Project Lead >> >> >> >> SpringSource - A Division of VMware >> >> >> >> http://www.springsource.com >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> --------------------------------------------------------------------- >> >> >> >> To unsubscribe from this list, please visit: >> >> >> >> >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> >> >> >> >> -- >> >> >> Graeme Rocher >> >> >> Grails Project Lead >> >> >> SpringSource - A Division of VMware >> >> >> http://www.springsource.com >> >> >> >> >> >> >> >> >> --------------------------------------------------------------------- >> >> >> To unsubscribe from this list, please visit: >> >> >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> -- >> >> Graeme Rocher >> >> Grails Project Lead >> >> SpringSource - A Division of VMware >> >> http://www.springsource.com >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe from this list, please visit: >> >> >> >> http://xircles.codehaus.org/manage_email >> >> >> >> >> > >> >> >> >> -- >> Graeme Rocher >> Grails Project Lead >> SpringSource - A Division of VMware >> http://www.springsource.com >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > -- Graeme Rocher Grails Project Lead SpringSource - A Division of VMware http://www.springsource.com --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
«
Return to Grails - dev
|
1 view|%1 views
Loading...
| Powered by Nabble | Edit this page |