Skip to content

HHH-12556 Share data structures between similar LoadPlan based EntityLoaders #2277

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

Merged
merged 1 commit into from
May 9, 2018

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented May 9, 2018

This is part of a series of patch to improve our memory consumption when dealing with legacy batch fetch style.

With the Openbravo test case contributed by the OP, this change alone brings the memory used from 1002 MB to 461 MB.

I think this change is pretty safe API wise, that's why I isolated it.

@gsmet gsmet requested review from Sanne and sebersole May 9, 2018 13:34
@Sanne Sanne self-assigned this May 9, 2018
@Sanne
Copy link
Member

Sanne commented May 9, 2018

Looks great, I'd like to merge it.

@gsmet
Copy link
Member Author

gsmet commented May 9, 2018

@Sanne btw, I'm wondering if we should consider backporting it to 5.2.

@Sanne
Copy link
Member

Sanne commented May 9, 2018

re: backporting. As you prefer. It makes sense to push some safe improvements to 5.2 so to have a larger pool of testers quickly, yet I also believe we should give motivation to ecnourage more aggressive updates.

Copy link
Member

@sebersole sebersole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me. WRT 5.2 - +1

@gsmet gsmet merged commit 3834c4d into hibernate:master May 9, 2018
@gsmet
Copy link
Member Author

gsmet commented May 9, 2018

@Sanne @sebersole merged and currently backporting to 5.2.

@gbadner once this one gets a bit of field testing, it might be a good idea to backport it to 5.1.

@Sanne
Copy link
Member

Sanne commented May 14, 2018

awesome! thanks @gsmet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants