Optimization Suggestions

[CryptoMorin]

All the links are taken from the experimental branch.

EntityTracker

This one is the most important one as it's frequently used. https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/EntityTracker.java#L103

private static final HashSet<EliteMobEntity> eliteMobs = new HashSet<>();
public static EliteMobEntity getEliteMobEntity(Entity entity) {
    if (!EliteMobProperties.isValidEliteMobType(entity)) return null;
    for (EliteMobEntity eliteMobEntity : eliteMobs)
        if (eliteMobEntity.getLivingEntity().equals(entity))
            return eliteMobEntity;
    return null;
}

Here you're looping thro all the mobs in the set to get your entity that you want, this completely defeats the purpose of having a "hash"set/map. You want it to be O(1) not O(n) (if you don't know what the O stuff means Google Big O notation) not to mention that looping thro a set is much slower than lists and arrays. Here, you want to switch to a HashMap, which is technically a HashSet, but with different key and value types.

private static final Map<Integer, EliteMobEntity> eliteMobs = new HashMap<>();
public static EliteMobEntity getEliteMobEntity(Entity entity) {
    return eliteMobs(entity.getEntityId());
}

Don't forget to do the same thing for methods accessing npcEntities as well.

That's it. You don't need to check if the entity is valid or anything else. It'll return null anyway. You don't even need to use the UUID of the entity since you're not doing anything with the UUIDs. With that being said, you can just remove this. It's redundant.

private static final HashSet<LivingEntity> eliteMobsLivingEntities = new HashSet<>();

Change isEliteMob() to check eliteMobs's map instead and remove the isValidEliteMobType() check. Note you don't need to use isEliteMob() if you're going to use getEliteMobEntity() after it.

And to register the mobs, you just put them in the HashMap with their LivingEntity#getEntityId() as the key and the instance of EliteMobEntity as the value.

---

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/EntityTracker.java#L368 Consider rewriting this method. At least add a boolean check to your unregister methods so you can stop the code from doing extra checks if the last unregister was successful.

---

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/EntityTracker.java#L405 In this method every time you get an element from the iterator you check for nulls. HashSet can only have one null, but HashMap can have many null values (but only one null key.) You shouldn't check for nulls anyways, because if an element is null, it's simply a programming error, it'd be your fault that you added the null.

---

SuperMobProperties

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/mobconstructor/mobdata/passivemobs/SuperMobProperties.java#L34 Change this method using the same way above.

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/mobconstructor/mobdata/passivemobs/SuperMobProperties.java#L13 Use an EnumSet instead. I don't think you'll even need this once you fix superMobData map.

---

EliteMobEntity

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/mobconstructor/EliteMobEntity.java#L520 This doesn't really have anything to do with performance, but you're using getEquipment() too many times. Just save it to an object and use that object. I feel like this is useless, but if you want to micro-optimize:

• It reduces the file size and the generated code.
• The code basically has to call a method every time.
• For the sake of your own sanity when writing the code.

But instead of setting the equipments multiple times, make 4 ItemStack objects and once the if checks are over, just use them one time to set the equipment. This will avoid sending packets multiple times.

---

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/mobconstructor/EliteMobEntity.java#L732 Instead of a set, use a HashMap with the key as the power's file name or name (preferably the file name since Idk if the name can change or not or they tend to be longer) and the value as an ElitePower. Then you can just do powers.contains(power.getFileName()) Or override ElitePower hashCode() and equals() to compare the file name string.

---

WeightedProbability

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/utils/WeightedProbability.java#L12 Just loop thro values() instead of keySet()

---

NonSolidBlockTypes

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/utils/NonSolidBlockTypes.java#L9 Right now this is O(n). Make it a constant (add final modifier) and use an EnumSet instead. It's faster. You don't need initializeNonSolidBlocks() method either, use:

EnumSet.of(one, two, three, four, ...);

If you want it to be more memory efficient and faster, simply use a switch statement.

---

Frequent

These apply to a lot of classes:

• Don't use equals() for enums. It doesn't necessarily improve performance. It's mostly used to avoid NPEs and for compilers to catch syntax errors correctly.
• You might want to consider using Apache's StringUtils.split(String, char) when splitting strings by a single character. DONT someString.split(","); DO StringUtils.split(someString, ','); Java's default split method uses RegEx and yeah...
• When comparing worlds don't use equals() for some reasons Spigot's equals() doesn't compare worlds correctly. Simply just do a reference check world == anotherWorld because all the world instances come from a single Map which is in CraftBukkit.
• If you're doing event.isCancelled() return; right at the beginning of the event, just use ignoreCancelled = true parameter for @EventHandler And yes, it improves performance due to how Bukkit calls events.
• There is no need to clone getLocation() and getEyeLocation() They're copies.
• Don't create new Vector objects just to use Location#add(Vector) use Location#add(double x, double y, double z) instead.
• If you're going to remove an element from a map/set, don't use Map#containsKey or Set#contains do Map#remove(element) != null or Set#remove (which returns a boolean). Same goes for Map#containsKey and Map#get().

---

PlayerScanner

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/utils/PlayerScanner.java#L12 There is World#getNearbyEntities(Location, double, double, double) Just pass your range variable to all those double parameters.

---

TeleportTag

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/combatsystem/combattag/TeleportTag.java#L40 Player#getLocation()#toVector()

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/combatsystem/combattag/TeleportTag.java#L97 Don't use containsKey() then remove(), check what remove() returns

---

NPCProximitySensor

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/npcs/chatter/NPCProximitySensor.java#L28 Can't you just remove them directly from the original set if they don't pass the check?

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/npcs/chatter/NPCProximitySensor.java#L61 The same thing. Checking if it contains and getting it. Don't do this. This is O(2n) (with the current EntityTracker methods. Normally it'd be O(2))

---

PlayerData

https://github.com/MagmaGuy/EliteMobs/blob/experimental/src/main/java/com/magmaguy/elitemobs/playerdata/PlayerData.java I see a lot of containsKey() and get() stuff here. It's important to optimize these since it's the player data.

---

ElitePower

https://github.com/MagmaGuy/EliteMobs/blob/experimental/src/main/java/com/magmaguy/elitemobs/powers/ElitePower.java You're creating two new objects here for two different sets. A waste of memory. From what I saw in power classes, there is nothing that can be changed to really make a new object for them.

---

VersionChecker

https://github.com/MagmaGuy/EliteMobs/blob/52633fe93f5c9d7d5a73bfd5478569531c60b233/src/main/java/com/magmaguy/elitemobs/utils/VersionChecker.java#L7 I noticed you're using this a lot too. Instead of checking every single time, parse the actualSubVersion integer (which we call the minor version number) as a constant and then you can compare that number with anything you pass to the method. And you don't need to compare both major and minor numbers. Do you see Minecraft v2 happening any time soon? To initialize it statically, do this:

private static final int VERSION;
static {
    String version = Bukkit.getVersion();

    // If the index is not found, there's something wrong with the server.
    int index = version.lastIndexOf("MC:");
    version = version.substring(index + 4, version.length() - 1);

    // 1.13.2, 1.14.4, etc...
    int lastDot = version.lastIndexOf('.');
    if (version.indexOf('.') != lastDot) version = version.substring(0, lastDot);

    VERSION = Integer.parseInt(version.substring(2));
}

I really didn't check every single class. Just pointed out the ones I could notice when sliding thro the code.

[MagmaGuy]

Just a heads up, I've started working on implementing some of the suggested fixes. I've not had the time to really digest every suggestion but the ones I have analyzed I can 100% say they are quality remarks and improvements.

It's pretty clear now that a lot of these systems were incorrectly overhauled through various updates, the entity tracker one is a prime example of complete nonsense. This is not the excuse the nonsense code, just to perhaps try to explain how the hell things like that hashset came to be.

Like I mentioned in Discord, I will slowly be trickling down the list, incorporating some variation of these fixes as I go along. Thanks for the quality work here Crypto.

[CryptoMorin]

No problem ^^ I kinda feel like I explained some of these in a half-assed way with some grammatical errors. If you couldn't understand anything, I'll try to rephrase it in a better way for you.

[MagmaGuy]

Alright so I think you'll find that I took to heart making optimizations to the EntityTracker in the latest version. I spent about 150 hours in total tearing out the old structures and rebuilding it all to be as performant as I could make it from the ground up. Let me know what you think.

(treasure chests and phase bosses haven't really been updated yet)

[CryptoMorin]

There's a slight memory issue with the new entity inheritance system. You're extending TrackedEntity that holds the uuid of the entity, but in your extended classes, you're declaring another uuid that holds the same entity uuid. This creates two UUID references (Why? Because what if you try to access super.uuid in EliteEntityTracker for example?) They'll point to the same object, but it's still one redundant reference. Also, you're saving another uuid object inside of your EliteMobEntity that's 3 references now.\ Instead of all of this, just make a getUniqueId() that gets the uuid directly from your LivingEntity inside EliteMobEntity and don't worry, that method itself references a cached uuid object, so you have 4 uuid objects that are the same :P

https://github.com/MagmaGuy/EliteMobs/blob/6f72e5ae0e305651851fdea8da2e3924da213876/src/main/java/com/magmaguy/elitemobs/entitytracker/EliteEntityTracker.java#L15

https://github.com/MagmaGuy/EliteMobs/blob/6f72e5ae0e305651851fdea8da2e3924da213876/src/main/java/com/magmaguy/elitemobs/entitytracker/TrackedEntity.java#L23

https://github.com/MagmaGuy/EliteMobs/blob/6f72e5ae0e305651851fdea8da2e3924da213876/src/main/java/com/magmaguy/elitemobs/mobconstructor/EliteMobEntity.java#L90

https://github.com/Bukkit/mc-dev/blob/c1627dc9cc7505581993eb0fa15597cb36e94244/net/minecraft/server/Entity.java#L78

These redundant references are just gonna pile up for a lot of entities. It's not a serious issue at all, it's just something that I thought might be worth mentioning. You can keep it like that if you want.

[MagmaGuy]

It's a fair point, I was aware of the redundancy and was originally going to do this differently but the actual implementation ended up not requiring it. I've removed the redundancy for the latest version.

I've also removed the memory watchdog, hopefully it simply is not necessary anymore (though I am still working on it actively to make sure it doesn't result in a memory leak).

[MagmaGuy]

Implemented & optimized, thank you for your service!