Good plugin performance

Discussion in 'Plugin Development' started by black_ixx, Dec 5, 2012.

Thread Status:
Not open for further replies.
  1. Offline

    black_ixx

    Well Ive created a very big plugin with mutliple features. It is able to store different things and I want it not to need much CPU. First Ive created different storagefiles for different things like money. This wont work good because when the file is to big there will be some bugs. So Ive created a MySQL-class which works fine. But now I have another better idea. So the plugin stores very much different things about each player -> I could create one storage per player. Ive started to create the mainclass which manages this storage files:

    Code:
    package org.black_ixx.war.playerStorage;
     
    import java.io.File;
    import java.io.FileNotFoundException;
    import java.io.IOException;
     
    import org.black_ixx.war.War;
    import org.bukkit.configuration.InvalidConfigurationException;
    import org.bukkit.configuration.file.YamlConfiguration;
     
    public class PSMain {
     
        private static War plugin;
        //private static File file;
        //private static YamlConfiguration config;
        static String path = plugin.getDataFolder().getAbsolutePath()+ "/players/";
     
        public static void init(War pp) {
            plugin = pp;
           
        }
     
        public static void addPlayer(String name){
            File f =new File(plugin.getDataFolder().getAbsolutePath()
                    + "/players/"+name+".yml");
            YamlConfiguration config =YamlConfiguration.loadConfiguration(f);
            config.set("WP", 350);
            save(config, f);
        }
        public static void setWP(String name, int wp){
            YamlConfiguration config = YamlConfiguration.loadConfiguration(new File(path+name+".yml"));
            config.set("WP", wp );
        }
        public static int getWP(String name){
            return (YamlConfiguration.loadConfiguration(new File(path+name+".yml")).getInt("WP"));       
        }
     
        public static void save(YamlConfiguration config, File file) {
            // Set config
            try {
                // Save the file
                config.save(file);
            } catch (IOException e1) {
                plugin.getLogger().warning(
                        "File I/O Exception on saving storage.yml");
                e1.printStackTrace();
            }
        }
     
       
       
       
        public static void reload(YamlConfiguration config, File file) {
            try {
                config.load(file);
            } catch (FileNotFoundException e) {
                e.printStackTrace();
            } catch (IOException e) {
                e.printStackTrace();
            } catch (InvalidConfigurationException e) {
                e.printStackTrace();
            }
        }
       
    }
    
    But the plugin has to choose every file again everytime when anything was changed. Is this ok or does it need to much CPU? Do you know how to make this better?

    Tips are welcome!
     
  2. Offline

    zeeveener

    Why would you parse every file when one thing is changed for a single player? Why not just search the folder for the file, then handle the result properly? Essentials does this very well, try looking at their source for some help also.

    By the looks of your code, you don't seem to be doing anything too intense. Just make sure that there are no Global variables in the player files. If there are, move them to a specific config file so that you aren't forced to change a setting in every file...
     
    black_ixx likes this.
  3. Offline

    black_ixx

    Kk. Thanks Ill take a look. Also the most classes of my plugin contain static methods. Is that bad? Or does'nt matter?
     
  4. Offline

    zeeveener

    I don't really know to be honest. I don't ever make anything static. No real reason why, just never knew why I should if I should. I have heard, however, that static methods and variables can cause memory leaks which WILL take up way more memory.

    I'd just go into Eclipse or Netbeans and tell it to automatically set things to static if they are supposed to be that way.
     
  5. Offline

    theguynextdoor

    By 'very big plugin' How big are we talking? 1000 lines, 2000, 3000, more?

    What i would do in your case is actually create a 'User' class, and store all relevant information for each user in that class. So it could look like this.

    Code:
    public class User {
      private War plugin;
      private String name;
      private File fileLoc;
      private FileConfiguration userFile;
     
      public User (War instance, String name) {
          plugin = instance;
          this.name = name;
          fileLoc = new File(plugin.getDataFolder() + File.separator + "Players", name + ".yml");
          userFile = YamlConfiguration.loadConfiguration(fileLoc);
      }
     
      public String getName() {
           return name;
      }
    }
    Then to keep track of them, you would have a Map in your main class which would look like this.

    Code:
    public class War {
      private Map<String, User> users;
     
      public void onEnable() {
        users = new HashMap<String, User>();
      }
     
      public void addUser(String name) {
          if (!users.containsKey(name)) {
              User user = new User(this, name);
              users.put(name, user);
          }
      }
     
      public User getUser(String name) {
        return users.containsKey(name) ? users.get(name) : new User(this, name);
      }
    }
    Then if you want to go one step above that, let the class implement ConfigurationSerializable and then make the serialize method include all of the data you want to save and then it is much easier to just do configurationFile.set(path, user.serialize);

    Something along these lines. You will need to make this code for what you want. I just coded this from the top of my head in this text box so things are likely to be off/incorrect in the syntax. But the theory is there.
     
    black_ixx likes this.
  6. Offline

    black_ixx

    Well its full size is 936 kb (compressed 506 kb). Its currently my biggest plugin ever.

    Your code looks quite helpful! Thanks :D Ill try this out. Also Ill add a map which stores the money and other things of the users and save it every x seconds at the storage. Then its alot faster to get the information and change the money.
     
  7. Offline

    theguynextdoor

    936 or even 506 seems quite big ... almost too big. My Tribes plugin is ~5000 lines of code and is only 144.33KB. I guess it also depends on amount of classes.

    If you use eclipse, try installing this plugin

    https://developers.google.com/java-dev-tools/download-codepro

    if you don't have eclipse ..... then find an equivalent.

    Trust me when i say that the majority of big plugins which hold data for each user tends to have a class dedicated to each user holding such data.
     
  8. Offline

    black_ixx

    Well its very big because it has many many different features like an unique war, zombiearena, spleef, adventuremode, survival etc. Its the core of a new server.
     
  9. Offline

    theguynextdoor

    Sounds interesting, if you wish for me to take a look to see if any improvements/changes could be made then upload it to github and ill take a look. Otherwise, i wish you the very best in your quest for making, what seems like, a moderately sized plugin
     
  10. Offline

    TheE

    Saving it every x seconds is pretty much useless. One the one hand you might ending up saving more often then it would be needed as because nothing has changed, on the other hand you might lose data if the server is crashing before changed stuff has been saved. And you are also taking much more performance than you need, as you are saving everything anytime, even if just one value has changed.
    The better way of doing this is to safe only changes and only the values that were actually changed.

    In addition to that, I would recommend you to set up your data connections by creating an interface with every needed method to handle the data connection first, and than write that actuall data connection e.g. MySQL by extending that interface. This might look complicated at first, but this methods allows you to add other storage options like YAML or SQLite later without forcing you to rewrite your whole code.
     
  11. Offline

    black_ixx

    I might upload the source when Im done with my ToDo list. Thanks for your offering :D


    How to save only one changed line instead of savind the whole config?

    Also I think its faster to create one map which stores money and saves it after x seconds. That should be faster as getting the information with a long code and the config. And the money-actions should be really fast because at one game, everytime when a player hits another player he gets money. If the code would be long and more then 20 players are hitting each other around 16 times per second it wont be good.



    Well now Ill start creating the config part with theguynext's tips.
     
  12. Offline

    jayfella

    There are various reasons to make something static, for example, a directory path, which will never change would probably be a static final String. Primarily static objects should not interfere with anything outside their surroundings. That is to say - it will "take" values, and use those to return a value, but not modify anything directly. For example, i have 2 int's - i want to multipy them and return a value. I DO NOT modify the values given to the static method. EVER. I USE them to return a completely new int. It does not directly modify any value, only yields a completely new result. If your method does that - then it can be considered a candidate for a static method. If it modifies something directly, such as a file, a value, etc, etc.. then it should not be static. That is pretty much rule #1 for static methods/properties - however generally you should try to keep everything non-static and put "helper" methods grouped in classes.

    What you want to do is store the data in variables - in a sense cacheing it - and finally dump it on completion of an event or when you know the rate data of reading/writing is at its lowest. For example, when spleef is done. Constantly reading/writing files or thrashing a database is not cool and you will just push yourself into another I/O corner you could have avoided by carefully thinking about what you are doing before writing it.
     
  13. Offline

    TheE

    Depends on the the way you safe data. If you are using MySQL you would simply run an normal update statement. In case of YAML you would simply set the value like you would do when saving everything.

    I think you do not understand what I meant. You have a HashMap containing player's and there money and you use this HashMap if you want to get how much money a player has. Whenever a value is changed you update it in the HashMap and you save the changes back to your database. That is the way most major plugins who deal with databases work.

    Everything else will coast you more performance and might result in data loss for the reasons I described above.
     
  14. Offline

    black_ixx

    Yeah Im talking about the same way. Storing data in hashmap and prevent losing the data when the server crashs by saving the hashmap sometimes at the config.


    Im using YAML and the problem is, that the config.set("path", value); method only changes the line without saving it. This the line would get lost when the server stops without saving the line.


    I never (almost) use static variables, but my methods look like this:
    Code:
        public static void spawnZombie(Location loc, int type, int lives, int price)
        {
            Zombie z = (Zombie) loc.getWorld().spawnEntity(loc, EntityType.ZOMBIE);
            zombieTypeStore.put(z, type);
            //z.setHealth(lives);
            ZombieListenerDamage.livesStore.put(z, lives);
            ZombieArenaPrice.entityMoneyStore.put(z, price);
        }
    Some months ago Ive started with the plugin and Ive created a static method. Henceforward Ive continued setting my new methods to static. Ive also tested the plugin and it works good but Im not sure if there is a better way than the static methods.
     
  15. Offline

    jayfella

    Static obviously means "never changing" - but you are changing things, so by its very nature, is not static. You won't notice the problems this can cause until you hit a problem and can't figure out why the hell its not working as it should. Instead you should create an instance of the class and call the methods from the class:

    Code:
    MyClass myclass = new myClass();
     
    myClass.spawnZombie();
     
    
    You will most likely want to only have one instance of this class and not have to create it everywhere you need it, so you pass it by reference via the constructor wherever you need it. You can even go hardcore and ensure it's a singleton. It's difficult to explain over a single post on how to go about doing things like this but basically you want a "parent" class where all of your common "child" classes are initialized - for example - your plugin is your "parent class" - and so you initialize your children inside it. You can then pass your plugin via the constructor to anywhere and automatically have access to everything inside the plugin. This ensures that only one instance is ever needed of everything you use often, and more importantly, you significantly reduce object creation, which can be intensive. It also makes for neat, tidy code and of course maintenance. Sorry if it comes across as intimidating, but that's how you should generally lay things out. Its long and boring to lay the ground - as it were - but thems are the jobs :p
     
  16. Offline

    fireblast709

    Just fixing some things:
    • static does not mean never changing, that is called 'final' (as you could, for instance, use static to count the number of objects you create
    • Afaik a singeton is something else, more like:
    Code:java
    1. class singleton
    2. {
    3. public static s = new singleton();
    4.  
    5. private singleton()
    6. {
    7. }
    8.  
    9. public static getSingleton()
    10. {
    11. return s;
    12. }
    13. }
     
  17. Offline

    jayfella

    final equates to "readonly" as far as I am aware - after its initialization, but i meant "never changing" as in not changing the given objects, properties or anything else from inside the static method.

    A singleton just ensures that only one instance of the object can ever exist - or at least tries to - there are various implementations and ways to overcome them. I was just explaining that rather than create an instance of a class just so you can access a method constantly, you can create a singleton "context" and initialize the class from there, and thus be able to access it from everywhere. Its hard to explain complex situations and layout guides in a simple post - my apologies if it comes across as confusing :p I probably shouldn't have said anything at all since a lot of these things are "useless" unless the user has a need for them - then again - the user may never know they exist without offering the idea - catch 22 :p
     
  18. Offline

    TheE

    Than you won't have changes in your files that occurred between the last save and the crash.

    So you need to save the config file after changing the value - where is the problem? That is still much better than saving everything when it is not necessary.
     
  19. Offline

    black_ixx

    I dont think that the server will crash but it would be better to loose some new changes than saving the whole playerConfig everytime, right? Or would you just save one line (Is this even possible?)? Every second there are many config-changes. I dont think that a repeating schedule which saves the changed configs would be not necessary.



    First my idea was to create two maps which store the two types of money of the players. My new idea was to add the money to the "User" files. Now my code is:



    Code:
    package org.black_ixx.war.playerStorage;
     
    import java.io.File;
     
    import org.black_ixx.war.War;
    import org.bukkit.configuration.file.FileConfiguration;
    import org.bukkit.configuration.file.YamlConfiguration;
     
    public class User {
      private War plugin;
      private String name;
      private File fileLoc;
      private FileConfiguration userFile;
      //private ConfigurationSerializable c;
      private int wp;
      private int total;
     
      public User (War instance, String name) {
          plugin = instance;
          this.name = name;
          fileLoc = new File(plugin.getDataFolder() + File.separator + "Players", name + ".yml");
          userFile = YamlConfiguration.loadConfiguration(fileLoc);
          wp = getConfig().getInt("WP");
          total = getConfig().getInt("WP");
      }
     
      public String getName() {
          return name;
      }
      public FileConfiguration getConfig(){
          return userFile;
      }
      public int getTotal(){
          return total;
      }
      public int getWP(){
          return wp;
      }
      public void setWP(int a){
          wp= a;
      }
      public void setTotal(int a){
          total=a;
      }
    }

    Another class allows me to work with "User":

    Code:
    package org.black_ixx.war.playerStorage;
     
    import java.util.Map;
     
    import org.black_ixx.war.War;
     
    public class PSHelper {
        private static War plugin;
        public static void init(War p)
        {
            plugin = p;
        }
     
     
          public static User getUser(String name) {
                return users.containsKey(name) ? users.get(name) : new User(plugin, name);
              }
              public static void addUser(String name) {
                  if (!users.containsKey(name)) {
                      User user = new User(plugin, name);
                      users.put(name, user);
                  }
              }
              public static Map<String, User> users;
    //          public static Map<String, Integer> normalWP;
    //          public static Map<String, Integer> totalWP;
    }
    
    And when someone gets WP this code is executed:

    Code:
        public void give(String playername, int amount) {
            User a =PSHelper.getUser(playername);
            a.setWP(a.getWP()+amount);
            //Prestige
            PrestigeAPI.give(playername, amount);
        }
    Also I think I will add the schedule which saves the WP into the storage soon.

    Ive optimized it a bit and it works perfect.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: May 30, 2016
  20. Offline

    Milkywayz

    It's nice to see people attempt to optimize their code, makes your stuff much higher quality.
    I will be willing to revise some stuff if you link me to the github, I would personally say my coding style is readable and minimal.
    One example of my work.
     
Thread Status:
Not open for further replies.

Share This Page