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

FacadeItemModel does not clear baseModel cache when a resource reload is requested #3370

Closed
asiekierka opened this Issue Feb 4, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@asiekierka

asiekierka commented Feb 4, 2018

What this causes is multiple instances of ModelLoader (as referenced by ModelLoader$VanillaModelWrapper, as it holds a reference to the parent class) to be kept in memory as soon as something requests a second resource refresh, causing greatly increased additional memory load.

"private IModel baseModel" should be set to null in an event which is called before model baking but during every resource reload. TextureStitchEvent.Pre might be a good place for it.

@yueh

This comment has been minimized.

Member

yueh commented Feb 4, 2018

Thanks. Actually ModelLoaderRegistry.getModel() appears to use a cached now, so it should be fine to use it instead of lazy evaluated field. The overhead should not be important compared to whatever else is done during model baking and it is certainly better than relying on some event to try and cleanup something vanilla/forge produced.

@yueh yueh added this to the rv5.stable - 1.12 milestone Feb 4, 2018

@yueh yueh added the type-bug label Feb 4, 2018

@asiekierka

This comment has been minimized.

asiekierka commented Feb 4, 2018

Yeah, just using ModelLoaderRegistry.getModel() directly is fine.

Please fix it ASAP, though, as the size of the dangling ModelLoader can get pretty high - on the pack I've tested (280 mods) it caused a 1GB difference in memory usage, which was over 25% of its overall usage (with FoamFix - the ratio is smaller, but still considerable without FoamFix)!

yueh added a commit that referenced this issue Feb 4, 2018

@yueh yueh closed this in 6cffe93 Feb 5, 2018

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