Fix Forge Duping Class Cache
NullSector76 opened this issue · 23 comments
You say you want to free up the memory. This is the main issue with mods and it's due to the launch wrapper storing the byte array and class object one only the class one is needed past initial dev.
MinecraftForge/MinecraftForge#5532
What can you do. Every time a class is loaded on your new FMLCorePlugin with the sorting index of Integer.max_value clear the class resourceCache via reflection and input back the world class and bytes if available
You mean this?
no I mean LaunchClassLoader#resourceCache it contains a huge cache between ob class and byte array forge only uses the private array for one thing to check the world class bytes so empty the array all except for the world class.
So empty it when possible all except that one object and forge should be happy but, your ram to. There is no need to keep an unused obfuscated hashmap byte arrays loaded into memory
yeah I don't think it's good enough you clear the maps once in init it looks like? here is the proper way to keep a constant clean ob map
"What can you do. Every time a class is loaded on your new FMLCorePlugin with the sorting index of Integer.max_value clear the class resourceCache via reflection and input back the world class and bytes if available"
Also make sure your function to get not in use keys actually works for what you think it does and not look for is this a weak key instanceof of a weakhashmap? no ok do nothing.
Okay, so.
I don't think it's good enough you clear the maps once in init it looks like?
Most classes are loaded after then.
on your new FMLCorePlugin with the sorting index of Integer.max_value
Last I checked, you can only have one coremod per JAR? And this one can't just sit on Integer.MAX_VALUE; I wholly expect other mods to mess with its output.
clear the class resourceCache via reflection
That's slow. A better way would be to create a dummy Map<> which doesn't allow putting new values in (blindly consuming them), if you wanted to do it.
the world class and bytes if available
Why only the World class? How do you know there is no mod in existence relying on resourceCache in some manner?
Also make sure your function to get not in use keys actually works for what you think it does and not look for is this a weak key instanceof of a weakhashmap? no ok do nothing.
Admittedly, the resourceCache patch is a bit stupid - could be done better, but it gets the job done IIRC.
"Why only the World class? How do you know there is no mod in existence relying on resourceCache in some manner?" Open Call hierarchy many coremods, fml itself cache their isObf from LaunchClassLoader.getBytes(net.minecraft.world.World) So yeah you definitely need the world entry or mods hard coding it such as apple core and other coremods might crash from a wrong boolean. It would also screw up my coremods as they do the same thing because I copied a class from asm helper
a dummy map which would get then get populated where? pre-init before pre init on initialization of your class plugin/transformer? I like that idea alot better then letting it happen then overriding it.
When I work on my fix with a dummy map I will give you the code without needing to modify your transformer much at all
I'm not interested in this type of PR, sorry. FoamFix is largely done for 1.12.2 at this point, and I would rather not touch it for what is comparatively a miniscule gain - and if I do, I'd rather do it myself.
https://pastebin.com/BrSP2r5j very simple to fix your stuff do the 10 line of patches. Notice the Class Initializer that's when you should use your hooks for LaunchClassLoader. This worked on my end
After getting some sleep that's all you have to do nothing with hard coding an entry it will just fetch it when getting class bytes each time from the disk.
You shouldn't need a config for this as it will just fetch it from the disk each time a method manually calls getClassBytes() or when a new class not in memory get's transformed
hey do classloaders even need to store an array of cached classes. I just tested with both maps being the dummy map and my transformers only ran once I kept the game open for 20 minuets and I edit alot of classes.
If not you can clear the LaunchClassLoader#cachedClasses as well for an even bigger memory optimization.
https://pastebin.com/t5JHWatY
Ok so use reflection to disable your config option manually I guess that could work? I still recommend removing the code for future versions or use the code I sent you thanks for your time
If not you can clear the LaunchClassLoader#cachedClasses as well for an even bigger memory optimization.
It's not like the Class objects will disappear. The VM still has them.
Right so can you fix your fix? Make it clear both mappings in LaunchClassLoader to dummy maps that can't store anything I already gave you the code for both please apply the fix for 1.12.2 as it's 2 seconds for you and will save ram for modders.
With my coremod test your foamfix actually replaces resource cache then adds back memory objects that should have been clear. So fix your fix and make it use a dummy map
Code again. Put this in your transformer class doesn't matter which use your reflection util and call it a day.
https://pastebin.com/zd3MFmUv
2 seconds for you
At least ten minutes to start the game, stop the game, add the necessary try...catches, push the code and push the release to two mod distribution locations. And this doesn't even cover the (slim, but real) chance of something going wrong. (For instance, I had to ensure the code doesn't run with FastCraft 2 - as its LaunchClassLoader replacement does not provide a resourceCache field)
Why does the code not run with fastcraft? Launch.classLoader should always be the instance set.
Edit: O I see your point the field won't get set because they replaced it. Well my reflection util catches the exception and prints stacktrace that's all that happens for me you would have to do the same
If your implementation is so good, why not release it as a separate mod? :V
I'll get around to it when I have some time and aren't busy working on Fabric. The memory difference caused by it is very miniscule compared to the things FoamFix targets most, anyway.
because it's already in a mod Evil Notch Lib next beta at least. It's a massive library that fixes alot of bugs.
That's great! That means I don't have to do it, just like when FastWorkbench came along. Less work for me \o/
But, it's for your sake not everyone wants a massive library. Multiple overrides for one field will work across multiple mods and don't need to depend upon each other for this fix.
Also your code still breaks my code because your fix is improper replaces the mapping for resource cache and then adds objects back into the mappings rather then just keeping empty maps.
So if you don't want to do this fix properly please just remove the code as it breaks my fix because your fix doesn't clear everything and replaces the map with a different one and fires after mine