I think it's worth investigating the cause and see how we could address it in the next release.

I asked because someone might have an idea about some change that was doe in 5.3 that might affect it. For the moment, I haven't started doing anything about it. Vlad On Thu, May 3, 2018 at 2:01 PM, Guillaume Smet <guillaume.smet(a)gmail.com&gt; wrote: > On Thu, May 3, 2018 at 9:35 AM, Vlad Mihalcea <mihalcea.vlad(a)gmail.com&gt; > wrote: > >> I think it's worth investigating the cause and see how we could address it >> in the next release. >> > > Sure, something looks fishy there. > > Will you do it or do you expect someone else to do it? It's not very clear > from your email. > > -- > Guillaume > > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

I just looked at that thread and it's comparing 3.6 to 5.3.CR2... On Thu, May 3, 2018 at 4:17 AM, Vlad Mihalcea <mihalcea.vlad(a)gmail.com&gt; wrote: > I asked because someone might have an idea about some change that was doe > in 5.3 that might affect it. > For the moment, I haven't started doing anything about it. > > Vlad > > On Thu, May 3, 2018 at 2:01 PM, Guillaume Smet <guillaume.smet(a)gmail.com > > wrote: > > > On Thu, May 3, 2018 at 9:35 AM, Vlad Mihalcea <mihalcea.vlad(a)gmail.com > > > wrote: > > > >> I think it's worth investigating the cause and see how we could address > it > >> in the next release. > >> > > > > Sure, something looks fishy there. > > > > Will you do it or do you expect someone else to do it? It's not very > clear > > from your email. > > > > -- > > Guillaume > > > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

OK, let me know if I should jump in...

On Thu, May 3, 2018 at 10:59 AM, Steve Ebersole <steve(a)hibernate.org&gt; wrote: > Yeah, we have been having a chat about this on HipChat > > On Thu, May 3, 2018 at 12:54 PM Gail Badner <gbadner(a)redhat.com&gt; wrote: > >> I just looked at that thread and it's comparing 3.6 to 5.3.CR2... >> >> On Thu, May 3, 2018 at 4:17 AM, Vlad Mihalcea <mihalcea.vlad(a)gmail.com&gt; >> wrote: >> >> > I asked because someone might have an idea about some change that was >> doe >> > in 5.3 that might affect it. >> > For the moment, I haven't started doing anything about it. >> > >> > Vlad >> > >> > On Thu, May 3, 2018 at 2:01 PM, Guillaume Smet < >> guillaume.smet(a)gmail.com&gt; >> > wrote: >> > >> > > On Thu, May 3, 2018 at 9:35 AM, Vlad Mihalcea < >> mihalcea.vlad(a)gmail.com&gt; >> > > wrote: >> > > >> > >> I think it's worth investigating the cause and see how we could >> address >> > it >> > >> in the next release. >> > >> >> > > >> > > Sure, something looks fishy there. >> > > >> > > Will you do it or do you expect someone else to do it? It's not very >> > clear >> > > from your email. >> > > >> > > -- >> > > Guillaume >> > > >> > > >> > _______________________________________________ >> > hibernate-dev mailing list >> > hibernate-dev(a)lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

Hi, So, I analyzed the dumps yesterday evening. The problem is real, meaning his SessionFactory is consuming more than 1GB of memory for 600+ tables, some with a lot of attributes. So for sure, the model is a big one, but it would be nice to be more gentle with this type of configuration. I don't think it's something new to 5.3 though as it's not the first time we have this type of reports. From my observations, the problem mostly comes from: LegacyBatchingEntityLoader \_ loaders - EntityLoader \_ staticLoadQuery - EntityLoadQueryDetails \_ rootReturn - EntityReturnImpl The largest LegacyBatchingEntityLoader I have in the dump takes more than 2 MB. Keep in mind that with a batch size of 50, we have 13 EntityLoaders for batch sizes of 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 20 and 50, each loader taking ~ 200 KB. We discussed some ideas yesterday with Steve. I'll try to experiment a few ideas next week to see if I can come up with some mitigation. I'm not that familiar with the ORM source code so I'm full of crazy ideas to try - most of them will probably prove to be bad (Steve already shut some of them) but maybe one will survive and be beneficial to the problem at hand. I'll keep you posted if I can come up with something. -- Guillaume

On Fri, May 4, 2018 at 11:48 AM, Sanne Grinovero <sanne(a)hibernate.org&gt; wrote: > thanks for investigating. Steve in chat mentioned the option of > creating these EntityLoaders lazily: as while they need to be reused, > it's possible many of them are never really used. > > I think that's a very interesting observation, is that what you plan to > explore? > Yes, it's one of the things we should explore. > I see two difficulties: > - you'd need thredsafe code, yet we don't want barriers on this area. > Will probably need features such as lazy set, yet these would need > even more objects allocated if you're not careful. > - we always prefer fail-at-boot validation, so we'd need to make sure > even if you don't build them all, we can at least make sure we'll be > able to build them later. > Well, the idea is to prototype something on the memory usage front, then we can discuss further what we want to do with it and if it's worth refining it and spending more time on it. If we end up choosing this path, be sure we'll be bugging you for advice on concurrency and performance :). > Additionally there's the idea to see if any such Loader instance could > be reused - probably using a cache which we clear at end of bootstrap > - yet since Steve mentioned this would be a significant change let's > not try to push it in 5.3. > > Finally, a simple one: I have the impression that loader's Map could > be made into a plain array, or a custom structure which would then be > ideal to support lazy creation of loaders. With the exception of two > special strings - which will needt to be treated specially - all other > keys are enums. > Yeah, I'll play with a few ideas and see what I can do. I'm not yet sure if not knowing the code base will be a thing in my favor or not. At least I don't have preconceived ideas :]. > Whatever you choose to try, let's keep significant changes for 6 only :) > Steve was saying that this whole thing wouldn't be an issue in 6. -- Guillaume _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

On 4 May 2018 at 19:45, Gail Badner <gbadner(a)redhat.com&gt; wrote: > I don't think the batching strategies actually changed when we moved to load > plans. > > Is the problem that each loader is taking considerably more memory? Yes that's right. > > On Fri, May 4, 2018 at 5:41 AM, Guillaume Smet <guillaume.smet(a)gmail.com > > wrote: >> >> On Fri, May 4, 2018 at 11:48 AM, Sanne Grinovero <sanne(a)hibernate.org&gt; >> wrote: >> >> > thanks for investigating. Steve in chat mentioned the option of >> > creating these EntityLoaders lazily: as while they need to be reused, >> > it's possible many of them are never really used. >> > >> > I think that's a very interesting observation, is that what you plan to >> > explore? >> > >> >> Yes, it's one of the things we should explore. >> >> >> > I see two difficulties: >> > - you'd need thredsafe code, yet we don't want barriers on this area. >> > Will probably need features such as lazy set, yet these would need >> > even more objects allocated if you're not careful. >> > - we always prefer fail-at-boot validation, so we'd need to make sure >> > even if you don't build them all, we can at least make sure we'll be >> > able to build them later. >> > >> >> Well, the idea is to prototype something on the memory usage front, then >> we >> can discuss further what we want to do with it and if it's worth refining >> it and spending more time on it. >> >> If we end up choosing this path, be sure we'll be bugging you for advice >> on >> concurrency and performance :). >> >> >> > Additionally there's the idea to see if any such Loader instance could >> > be reused - probably using a cache which we clear at end of bootstrap >> > - yet since Steve mentioned this would be a significant change let's >> > not try to push it in 5.3. >> > >> > Finally, a simple one: I have the impression that loader's Map could >> > be made into a plain array, or a custom structure which would then be >> > ideal to support lazy creation of loaders. With the exception of two >> > special strings - which will needt to be treated specially - all other >> > keys are enums. >> > >> >> Yeah, I'll play with a few ideas and see what I can do. I'm not yet sure >> if >> not knowing the code base will be a thing in my favor or not. At least I >> don't have preconceived ideas :]. >> >> >> > Whatever you choose to try, let's keep significant changes for 6 only :) >> > >> >> Steve was saying that this whole thing wouldn't be an issue in 6. >> >> -- >> Guillaume >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

I *think* that most, if not all, of the loaders for a particular entity could be reworked to share most of the same information. IIRC, the only real differences are the batch size and the actual query that gets generated. I'm happy to look into this, but I don't want to duplicate what others are doing. It a bit difficult collaborating on this since our time zones don't overlap very well (and, unfortunately, I'm not a morning person). I have some other work I need to attend to today. Please let me know if I should dig into this next week. On Fri, May 4, 2018 at 12:30 PM, Guillaume Smet <guillaume.smet(a)gmail.com&gt; wrote: > Don't know about the history but with the entity I checked out in the > OP's case, each EntityLoader is approximately 200 KB. > > The entity contains a lot of columns so that's no surprise. > > What doesn't help is that I don't think the 13 EntityLoaders/entity share > a lot of things so they are just adding up. > > On Fri, May 4, 2018 at 9:15 PM, Sanne Grinovero <sanne(a)hibernate.org&gt; > wrote: > >> On 4 May 2018 at 19:45, Gail Badner <gbadner(a)redhat.com&gt; wrote: >> > I don't think the batching strategies actually changed when we moved >> to load >> > plans. >> > >> > Is the problem that each loader is taking considerably more memory? >> >> Yes that's right. >> >> > >> > On Fri, May 4, 2018 at 5:41 AM, Guillaume Smet < >> guillaume.smet(a)gmail.com&gt; >> > wrote: >> >> >> >> On Fri, May 4, 2018 at 11:48 AM, Sanne Grinovero <sanne(a)hibernate.org >> > >> >> wrote: >> >> >> >> > thanks for investigating. Steve in chat mentioned the option of >> >> > creating these EntityLoaders lazily: as while they need to be >> reused, >> >> > it's possible many of them are never really used. >> >> > >> >> > I think that's a very interesting observation, is that what you >> plan to >> >> > explore? >> >> > >> >> >> >> Yes, it's one of the things we should explore. >> >> >> >> >> >> > I see two difficulties: >> >> > - you'd need thredsafe code, yet we don't want barriers on this >> area. >> >> > Will probably need features such as lazy set, yet these would need >> >> > even more objects allocated if you're not careful. >> >> > - we always prefer fail-at-boot validation, so we'd need to make >> sure >> >> > even if you don't build them all, we can at least make sure we'll be >> >> > able to build them later. >> >> > >> >> >> >> Well, the idea is to prototype something on the memory usage front, >> then >> >> we >> >> can discuss further what we want to do with it and if it's worth >> refining >> >> it and spending more time on it. >> >> >> >> If we end up choosing this path, be sure we'll be bugging you for >> advice >> >> on >> >> concurrency and performance :). >> >> >> >> >> >> > Additionally there's the idea to see if any such Loader instance >> could >> >> > be reused - probably using a cache which we clear at end of >> bootstrap >> >> > - yet since Steve mentioned this would be a significant change let's >> >> > not try to push it in 5.3. >> >> > >> >> > Finally, a simple one: I have the impression that loader's Map could >> >> > be made into a plain array, or a custom structure which would then >> be >> >> > ideal to support lazy creation of loaders. With the exception of two >> >> > special strings - which will needt to be treated specially - all >> other >> >> > keys are enums. >> >> > >> >> >> >> Yeah, I'll play with a few ideas and see what I can do. I'm not yet >> sure >> >> if >> >> not knowing the code base will be a thing in my favor or not. At >> least I >> >> don't have preconceived ideas :]. >> >> >> >> >> >> > Whatever you choose to try, let's keep significant changes for 6 >> only :) >> >> > >> >> >> >> Steve was saying that this whole thing wouldn't be an issue in 6. >> >> >> >> -- >> >> Guillaume >> >> _______________________________________________ >> >> hibernate-dev mailing list >> >> hibernate-dev(a)lists.jboss.org >> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > >> > >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > >

As already mentioned, I plan to take a look at this next week. I also thought we could share most of the info between loaders but Steve was thinking it might not be possible without massive changes. I'll take a look and report back to all of you. I might be way over my head as I don't know the codebase very well but I already did a very similar work for HV with great results so it's worth a shot. -- Guillaume On Fri, May 4, 2018 at 10:31 PM, Gail Badner <gbadner(a)redhat.com&gt; wrote: > I *think* that most, if not all, of the loaders for a particular entity > could be reworked to share most of the same information. > > IIRC, the only real differences are the batch size and the actual query > that gets generated. > > I'm happy to look into this, but I don't want to duplicate what others are > doing. It a bit difficult collaborating on this since our time zones don't > overlap very well (and, unfortunately, I'm not a morning person). > > I have some other work I need to attend to today. Please let me know if I > should dig into this next week. > > On Fri, May 4, 2018 at 12:30 PM, Guillaume Smet < guillaume.smet(a)gmail.com&gt; > wrote: > >> Don't know about the history but with the entity I checked out in the >> OP's case, each EntityLoader is approximately 200 KB. >> >> The entity contains a lot of columns so that's no surprise. >> >> What doesn't help is that I don't think the 13 EntityLoaders/entity share >> a lot of things so they are just adding up. >> >> On Fri, May 4, 2018 at 9:15 PM, Sanne Grinovero <sanne(a)hibernate.org&gt; >> wrote: >> >>> On 4 May 2018 at 19:45, Gail Badner <gbadner(a)redhat.com&gt; wrote: >>> > I don't think the batching strategies actually changed when we moved >>> to load >>> > plans. >>> > >>> > Is the problem that each loader is taking considerably more memory? >>> >>> Yes that's right. >>> >>> > >>> > On Fri, May 4, 2018 at 5:41 AM, Guillaume Smet < >>> guillaume.smet(a)gmail.com&gt; >>> > wrote: >>> >> >>> >> On Fri, May 4, 2018 at 11:48 AM, Sanne Grinovero < sanne(a)hibernate.org >>> > >>> >> wrote: >>> >> >>> >> > thanks for investigating. Steve in chat mentioned the option of >>> >> > creating these EntityLoaders lazily: as while they need to be >>> reused, >>> >> > it's possible many of them are never really used. >>> >> > >>> >> > I think that's a very interesting observation, is that what you >>> plan to >>> >> > explore? >>> >> > >>> >> >>> >> Yes, it's one of the things we should explore. >>> >> >>> >> >>> >> > I see two difficulties: >>> >> > - you'd need thredsafe code, yet we don't want barriers on this >>> area. >>> >> > Will probably need features such as lazy set, yet these would need >>> >> > even more objects allocated if you're not careful. >>> >> > - we always prefer fail-at-boot validation, so we'd need to make >>> sure >>> >> > even if you don't build them all, we can at least make sure we'll be >>> >> > able to build them later. >>> >> > >>> >> >>> >> Well, the idea is to prototype something on the memory usage front, >>> then >>> >> we >>> >> can discuss further what we want to do with it and if it's worth >>> refining >>> >> it and spending more time on it. >>> >> >>> >> If we end up choosing this path, be sure we'll be bugging you for >>> advice >>> >> on >>> >> concurrency and performance :). >>> >> >>> >> >>> >> > Additionally there's the idea to see if any such Loader instance >>> could >>> >> > be reused - probably using a cache which we clear at end of >>> bootstrap >>> >> > - yet since Steve mentioned this would be a significant change let's >>> >> > not try to push it in 5.3. >>> >> > >>> >> > Finally, a simple one: I have the impression that loader's Map could >>> >> > be made into a plain array, or a custom structure which would then >>> be >>> >> > ideal to support lazy creation of loaders. With the exception of two >>> >> > special strings - which will needt to be treated specially - all >>> other >>> >> > keys are enums. >>> >> > >>> >> >>> >> Yeah, I'll play with a few ideas and see what I can do. I'm not yet >>> sure >>> >> if >>> >> not knowing the code base will be a thing in my favor or not. At >>> least I >>> >> don't have preconceived ideas :]. >>> >> >>> >> >>> >> > Whatever you choose to try, let's keep significant changes for 6 >>> only :) >>> >> > >>> >> >>> >> Steve was saying that this whole thing wouldn't be an issue in 6. >>> >> >>> >> -- >>> >> Guillaume >>> >> _______________________________________________ >>> >> hibernate-dev mailing list >>> >> hibernate-dev(a)lists.jboss.org >>> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>> > >>> > >>> _______________________________________________ >>> hibernate-dev mailing list >>> hibernate-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>> >> >> > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev