[Add-On] Memory Leak/Cache Pool Fix

[jon5477]

This is another fail attempt to revive a dead community. Anyways, the whole purpose of this fix is to save a hell lot of memory. Just do what I tell you to do.

Navigate to MapleItemInformationProvider (net.sf.odinms.server)

In MapleItemInformationProvider.java:

Change all:

new HashMap<>();

To:

new WeakHashMap<>();

WeakHashMap allows it so that certain entries that no longer have references will be garbage collected which will save you as much memory as possible.

Java API for WeakHashMap:
WeakHashMap (Java Platform SE 6)

Please read the API before implementing this fix.

 

[SuperLol]

It doesn't save you much memory at all. The caches are very, at least the ones in that class. They reach up to about 20 mb at most in the heap and that's a very high upper bound. I'm not even sure the point of a cache is to remove the unreferenced ones when it is used here.

A real memory fix would be to convert the wz usage in that class to xml. That has saved me much much more memory.

 

[jon5477]

It doesn't save you much memory at all. The caches are very, at least the ones in that class. They reach up to about 20 mb at most in the heap and that's a very high upper bound. I'm not even sure the point of a cache is to remove the unreferenced ones when it is used here.

A real memory fix would be to convert the wz usage in that class to xml. That has saved me much much more memory.

Yep I forgot to mention. WeakHashMap is useful when the keys within the HashMap lose their reference. I would also suggest using LinkedHashMap and the removeEldestEntry() method to remove certain objects that have become obsolete or irrelevant. WeakHashMap objects are removed only when the reference for the key have become obsolete. However I'm not entirely sure that this approach would be completely beneficial. I may just switch to removing the oldest entry.

 

[SuperLol]

Well yes, that's why this release is useless. You don't want to lose your cached values here. If you do want to clear things, then use map.clear() to accomplish this. I have a method that does that, it's just silly though IMO. It defeats the purpose of the cache.

So if you do want to clear caches, you should do something like
public void clear() {
// insert all cached maps and call clear method here
}

 

[jon5477]

Well yes, that's why this release is useless. You don't want to lose your cached values here. If you do want to clear things, then use map.clear() to accomplish this. I have a method that does that, it's just silly though IMO. It defeats the purpose of the cache.

So if you do want to clear caches, you should do something like
public void clear() {
// insert all cached maps and call clear method here
}

IMO when you just clear all the maps constantly it kinda defeats the purpose of a cache. So I suppose an implementation of a background Thread clearing the map every once in a while would do the trick?

 

[SuperLol]

The idea is if you remove anything from it, it's useless. I heard it was good to clear caches every few hours anyways though. I've never actually cleared my caches, but I kept that option open. The only useful wz file needed in that class is Character.wz. All the String.wz references should be removed. Some 40mb saved in v88.

I also don't get the point of running a thread devoted to clearing caches after a while. That's a waste of a timer.

 

[NemesisToKill]

The only useful wz file needed in that class is Character.wz. All the String.wz references should be removed. Some 40mb saved in v88.

But then how would you get the item names from the wz files?
Or do you just not get them because they're unnecessary?

 

[iCris]

I think it would be more beneficial to just have GM or Admin commands that are able to clear certain caches. Something like this doesn't and shouldn't be done often and to me manually doing it seems the best way. Also it's helpful if you make any modifications to an XML and want it to take affect without a server restart. The only cache that shouldn't really be cleared is the one in MapleMapFactory as that could cause multiple instances of the same map to occur, unless you tinker with the getMap method.

 

[NemesisToKill]

I think it would be more beneficial to just have GM or Admin commands that are able to clear certain caches. Something like this doesn't and shouldn't be done often and to me manually doing it seems the best way. Also it's helpful if you make any modifications to an XML and want it to take affect without a server restart. The only cache that shouldn't really be cleared is the one in MapleMapFactory as that could cause multiple instances of the same map to occur, unless you tinker with the getMap method.

Back when I used to periodically clear the map cache through an event script (Before I realized that my event scripts were killing my server's uptime by over 200%), I used to only clear maps that had 0 players in them, preventing multiple instances from occurring (this ran every 4 hours). Plus, if a map is popular and/or actively being used, there's no point in removing it from the cache.

And my corresponding command for individual maps warps everyone in the map to FM 1, and then warps them back after a few seconds, which also prevents the multiple instances. But I only use this for maps that legitimately need to be reloaded, as it is quite costly depending on the number of people in the map.

 

[SuperLol]

It is known that event scripts have plenty of memory leaks in them due to improper disposal of the event instances. Anyways, it is very very bad to clear the map cache in maplemapfactory even though that holds by far the biggest use of memory. There are somethings that will disappear (player shops, merchants), and then the players in the map will be come invisible, and there will be spawn issues.

 

[iCris]

It is known that event scripts have plenty of memory leaks in them due to improper disposal of the event instances. Anyways, it is very very bad to clear the map cache in maplemapfactory even though that holds by far the biggest use of memory. There are somethings that will disappear (player shops, merchants), and then the players in the map will be come invisible, and there will be spawn issues.

I've had numerous problems with NPCs and Hired Merchants disappearing from maps. I dealt with the NPC disappearances in a not so effective way, but I am still unsure about the Hired Merchants.

 

[SuperLol]

That's not the problem I'm talking about, but clearing the map cache will remove them too. The NPCs will respawn though, but the shops owned by the players will not and they will not be able to open them again as well.