Skip to content
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

Reduce Memory pressure of UpgradePath #5060

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ichttt
Copy link

@ichttt ichttt commented Mar 4, 2019

Hi there,
I did some memory analysis on large modpacks, trying to reduce the memory pressure.
I noticed that the HashMap in your DarkSteelUpgradeRecipeCategory is taking up around 150MB of heap space. If you look at the objects inside the map that take, you find that 45MB of the 150MB are just char arrays. This is because the String id can grow quite huge.
My solution is to simply remove the id, and cache the hashcode instead.
The equals check should not be much slower, as it fails fast for different input items

The result is that DarkSteelRecipeManager uses around 45MB less memory.
Some screenshots from eclipse MAT:
Without my changes:
old_dominator
old_retained
With the changes in this PR:
new_retained
new_dominator

Copy link
Member

@HenryLoenwind HenryLoenwind left a comment

Choose a reason for hiding this comment

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

Looks good, but let's have tterrag have a look at this, too.

@@ -188,6 +187,12 @@ public static void addAdvancedTooltipEntries(@Nonnull ItemStack itemstack, Entit
return result.isEmpty() ? "" : NullHelper.first(result.substring(1), "");
}

public static @Nonnull NNList<IDarkSteelUpgrade> getUpgrades(@Nonnull ItemStack stack) {
NNList<IDarkSteelUpgrade> list = UpgradeRegistry.getUpgrades();
list.removeIf(upgrade -> !upgrade.hasUpgrade(stack));
Copy link
Member

Choose a reason for hiding this comment

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

I worry that this O(n) code is going to slow down the JEI handler significantly. Have you profiled the difference?

HenryLoenwind added a commit that referenced this pull request May 8, 2019
This is a safer but not as efficient version of #5060
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants