Improve performance in Roster class

Smack Smack Dev
1

Hi all,

During a smack’s profiling task it emerged a bottleneck managing the roster items and precisly the getEntry(String) method.

I guess the cycle that scroll the entries (roster items) slow down the process, to improve such point I tried to change the algorithm for roster items managing.

I replace the entries type of CopyOnWriteArrayList<RosterEntry> with a TreeMap<String,RosterEntry>, the key is the username, the value the roster entry.

In this way the list is ordered and the research of a specific entry is fast:

public RosterEntry getEntry(String user) {
        if (user == null) {
            return null;
        }
        return entries.get(user.toLowerCase());
    }

instead of:

public RosterEntry getEntry(String user) {
        if (user == null) {
            return null;
        }
        String userLowerCase = user.toLowerCase();
        for (RosterEntry entry : entries) {
            if (entry.getUser().equals(userLowerCase)) {
                return entry;
            }
        }
        return null;
    }

This enhancement speed up the process and our applications over smack library.

What do you think about this update? I attach the source class.

Thanks.

Giancarlo Frison
Roster.java (37539 Bytes)

2

While I dont know the exact reasons for choosing CopyOnWriteArrayList, it has specific features that TreeMap does not have. Specifically CopyOnWriteArrayList is thread-safe, where TreeMap is not (at least, not by itself). The trade-off is that every mutable operation (add,del,etc) generates a copy of the array. Maybe one of the original authors wants to comment on this?

3

I agree with slushpupie that the Map should be thread safe. So why not replace the TreeMap with a ConcurrentHashMap?

I did some tests with the code provided by Giancarlo. The test iterates through the entries in my XMPP roster (containing 261 entries) and calls a getEntry on each entry. I repeated the test 10.000 times.

Using the original Smack Roster class the test took 12,5 seconds to complete. Using the ConcurrentHashMap based Roster class the test took 1,8 seconds to complete.

4

SMACK-235

=)