-
Notifications
You must be signed in to change notification settings - Fork 933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NH-3530 - memory when using default_batch_fetch_size #1316
Comments
It seems like this is the regression of following: a35aaa6 |
We wanted to start using default_batch_fetch_size, but after enabling it, our tests started to throw OutOfMememory exceptions. Investigation led me to this issue... This is the difference in memory usage in our case, we are using NH 5.2.3 with default_batch_fetch_size=20...withoutUnfortunately this issue prevents us from using this feature... |
You may use instead discrete |
The memory impact is due to the |
Related: #1904, which would be an intermediate solution, maybe insufficient for some applications (if they usually end up using most loaders during their lifetime). |
Additional details in #2427 |
We ran in this issue, too. We have got round about 300 hundreds web applications using NHibernate under the hood. The usage of the batch size would help us to avoid SELECT N+1-problems etc. Thus a solution similar to Hibernate would be really great. |
Hi All We're a sponsor of the firebird project (we sponsor practically the complete .net driver for firebird). We migrated from LLBLGenPro to NHibernate. But, we're facing a severe issue with the problem described above. We have 50 databases on a server and hundreds of tables. Batching is set to 20 or 100 depending on table. We don't set default_batch_fetch_size, but still it requires a few GB just to start the project. Is there somebody who can implement DYNAMIC batching on short term? We're willing to sponsor the project. kind regards Alexander |
Hi Alexander, I reviewed the code and to me it is obvious that the memory won't be released. I have a fix locally which I will provide next week. Kind regards, |
Hi Carsten Any progress on this topic about dynamic sql generation as opposed of keeping all these "in ()" statements cached. kind regards Alexander |
Hi Any luck on this? It is getting extremely urgent for us since it is actually freezing our servers all the time. many thanks Alexander |
As far as I know, no one works on this trouble currently. PR are welcome. |
We have a solution on this. In our next sprint (three weeks) we need to prove that this has the expected behaviour. The problem with the solution above is that the memory will not be freed. Hopefully we can provide a solution in first week of october. Kind regards |
Hi all How are you doing on this issue? Can I follow it up somewhere? many thanks Alexander |
Hi Alexander, The fix provided in https://github.com/nhibernate/nhibernate-core/pull/1904/files was unsufficient because the memory still increases. So we needed to introduce a factory, so that the loaders will be created by a factory and the garbage collector can free memory. The following shows the difference: The fix will hopefully available during the next two weeks. Sorry for the delay. We are very busy at the moment. Kind regards |
I guess it would work. But it's not very nice solution to re-create loaders on each load operation. So it's good only for custom forks. The proper way to solve it described here
@AlexanderMuylaert If it's still a deal I can port DYNAMIC batching. I will have time to work on it in 2 weeks and I think I would need 1 week to have it implemented (though you would still need to stick to custom fork for some time till it's reviewed by other members) |
I agree that the other implementation would be better. But our tests show that we can solve select N+1-problems and get better performance with that solution. Memory was also not increasing. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Fixes #1316 Co-authored-by: Alex Zaytsev <hazzik@gmail.com>
Kris Desmadryl created an issue — 12th September 2013, 20:45:09:
Filip Duyck added a comment — 13th September 2013, 7:29:56:
Filip Duyck added a comment — 16th July 2014, 13:13:52:
The text was updated successfully, but these errors were encountered: