Performance Tips

A PacketInterceptor processes ALL packets that go through the server, so performance is important here.

1.

A PacketInterceptor processes all packets twice. The first time the flag processed is false, second time its true. Only in the first case you can drop packets. You should add the following line to PacketFilterPlugin.interceptPacket():

if (processed) { return; }

performance gain: near 100%

2.

Your are using some Enums, ok. But why you convert them to string and compare with equals?! Enums are Singletons, you can safely compare the references. A String in Java has two bytes per character, internally you need some kind of loop to compare two strings. References are nothing else than Integers (4 bytes), which can compared directly by hardware!

3.

Why are you converting JIDs to Strings and parsing them again?

E.g. PacketFilterUtil.getComponent() could easily replaced with JID.getDomain();

4.

Avoid unnecessary method calls....e.g PacketFilterUtil.getDomain() does nothing else than calling getComponent(). Method calls are time consuming because you must create new Stackframe and store all local variables in it. Allocating memory is always expensive.

Class PacketFilter could implement the PacketInterceptor interface directly.

5.

Method PacketFilterUtil.isLocalUser(). This method is.....puh! Thats what you are doing there:

1. Loading all usernames from database and sort them by name. Slow if the database is remote! Sorting takes O(n * log n), retrieving takes O(n).

2. Comparing the Collection of user in linear time (O(n)) ....in average case you have to compare 50% of all usernames!

3. Each time you compare the username building the string username+"@"+serverName. It would be easier to try username.equals(jid.getNode())....

4. Why you don't test if the domain matches the local domain before comparing all usernames?

Better use this one here:

public static boolean isLocalUser(JID jid) {
     return xmppServer.isLocal(jid) &&
            userManager.isRegisteredUser(jid.getNode());
}

This method uses the UserCache which uses internally a HashMap. A HashMap supports searching in O(1), which means it doesn't matter how many users you have, it is always extremely fast. (Better would be a TreeMap here, a bit slower (O(log n)), but takes less memory.)

If the user was not found in cache, its retrieved from database, but not all users, only the single one you want (O(1))! After retrieving the user is now cached, so next time is much faster.

I assume you have never heard something of theory of computation, theory of complexity, O-notation and such things.

The variable n means always the amount of data, in this case the count of users. O(n) means you need n steps of a constant time to do that want. E.g. if n = 1,000,000,000 and each step takes 1 second, you will take 31 years to complete your mission. But if you have a better algorithm, which takes only O(log n) time, you can do the same work in 30 seconds. Best algorithm would be O(1), which take always this constant time.

You see, you can write a lot better programs, if you have a bit knowledge of complexity.

(This is only a simplified explanation of complexity, check wikipedia, google, ...)

After writing this I saw that PacketFilterUtil.isLocalUser() is never used...

See bug_report here:

http://www.igniterealtime.org/community/thread/31918

If I find time I will try it tomorrow on our testing server.