Help IconsTable loads data everytime

[arm4729]

Hello , ive found on forum this code and added it to my project , i am using it for shift + click and lucky chest feature , so that when you open a chest an htm window appears and tells you what item you obtained , count and adds the icon of the item. 

Problem is that everytime shift + click is requested or opening chest , all 10k icons loads and i get this message on gs log:

IconsTable: Succesfully loaded 9209 icons, in 17.0 Milliseconds.

 

the code im using is this:

 

package net.sf.l2j.gameserver.datatables;

import java.io.File;
import java.util.HashMap;
import java.util.Map;
import java.util.logging.Logger;

import net.sf.l2j.gameserver.GameServer;
import net.sf.l2j.gameserver.xmlfactory.XMLDocumentFactory;

import org.w3c.dom.Document;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;

public class IconsTable
{
    public static final Map<Integer, String> Icons = new HashMap<>();
    private static int count;
    private static long t0;
    private static double t;

    private static final Logger _log = Logger.getLogger(GameServer.class.getName());

    public void reload()
    {
        Icons.clear();
        parseData();
    }

    public static void parseData()
    {
       count=0;
        t0 = System.currentTimeMillis();
        try
        {
            File f = new File("./data/xml/icons.xml");
            Document doc = XMLDocumentFactory.getInstance().loadDocument(f);

            for (Node n = doc.getFirstChild(); n != null; n = n.getNextSibling())
            {
                if ("list".equalsIgnoreCase(n.getNodeName()))
                {
                    for (Node d = n.getFirstChild(); d != null; d = d.getNextSibling())
                    {
                        if (d.getNodeName().equalsIgnoreCase("icon"))
                        {
                            count++;
                            NamedNodeMap attrs = d.getAttributes();
                            Node att = attrs.getNamedItem("Id");
                            Node att2 = attrs.getNamedItem("value");
                            Icons.put(Integer.valueOf(att.getNodeValue()), String.valueOf(att2.getNodeValue()));
                        }
                    }
                }
            }
            t = System.currentTimeMillis() - t0;
            _log.config("IconsTable: Succesfully loaded "+count+" icons, in "+t+" Milliseconds.");
        }
        catch (Exception e)
        {
            _log.config("IconsTable: Failed loading IconsTable. Possible error: "+e.getMessage());
        }
    }

    public static String getIcon(int id)
    {

        if (Icons.get(id)==null)
        {
            _log.config("IconsTable: Invalid Icon request: "+id+", or it doesn't exist, Ignoring ...");
            return "null";
        }
        return Icons.get(id);
    }

    public static final IconsTable getInstance()
    {
        parseData();
        return SingletonHolder._instance;
    }

    private static class SingletonHolder
    {
        protected static final IconsTable _instance = new IconsTable();
    }
}

 

and when i want to get the icon i use this

IconsTable.getInstance().getIcon(ITEMID)) , what i need to know is that if there isn't any way to store all this 10k icons , and load them only once , not for every request to get icon.. 

 

 

EDIT: PROBLEM SOLVED , HERE IS THE FIX 

   public static String getIcon(int id)
    {

        if (Icons.get(id)==null)
        {
            parseData();
            _log.config("IconsTable: Invalid Icon request: "+id+", or it doesn't exist, Ignoring ...");
            return "null";
        }
        return Icons.get(id);
    }

    public static final IconsTable getInstance()
    {
        return SingletonHolder._instance;
    }

i had to remove parsedata(); from the getInstance() and add it after if(icons.get(id)==null) , hope that this won't slowdown server much

Edited August 31, 2021 by arm4729

[TGSLineage2]

You were invoking the parse Data method to fill the Map from the xml every time you made the query of the icon for its key, the first thing if it could have reduced the performance of your server.

[Zake]

This is not a fix, you should avoid parsing data with every getInstance() call which you already did and parse data once in a private constructor. Reload method should be called on admin commands/bypasses. Anyway, i'm locking this topic since you feel satisfied with this solution. In case you want to unlock send me a dm.