Very poor performance of latest Yarep/Security Code

[baszero]

Hi,

I performed some tests and detected a severe performance issue in Yarep/Security.

See
https://github.com/wyona/security/blob/master/src/impl/java/org/wyona/security/impl/yarep/YarepUserManager.java#L87

My test calls the following:
realm.getIdentityManager().getUserManager().getUsers();

as I don't want caching, the Yarep/Security correctly loads all users from the repository.

With an older code of yarep/security, the whole test took about 20min for 40'000 users.
With the latest code, it took 3.5 hours!

In my log, I can see the debug statement within the loop of the code (see URL above for the line with the debug statement):

2016-08-11 09:25:48,388 YarepUserManager.loadUsersFromRepository():87 - User (re)loaded: dyntest978.xml, dyntest978
2016-08-11 09:25:48,778 YarepUserManager.loadUsersFromRepository():87 - User (re)loaded: dyntest979.xml, dyntest979
2016-08-11 09:25:49,169 YarepUserManager.loadUsersFromRepository():87 - User (re)loaded: dyntest98.xml, dyntest98
2016-08-11 09:25:49,579 YarepUserManager.loadUsersFromRepository():87 - User (re)loaded: dyntest980.xml, dyntest980
2016-08-11 09:25:50,000 YarepUserManager.loadUsersFromRepository():87 - User (re)loaded: dyntest981.xml, dyntest981
2016-08-11 09:25:50,422 YarepUserManager.loadUsersFromRepository():87 - User (re)loaded: dyntest982.xml, dyntest982

You see that it takes 500ms in average to just create the Yanel User object from a file. Too long!

My questions are:
Can you guess what could take so long?
If I look at the code within the loop that iterates over all users, there are two lines that could be slow:

a)
https://github.com/wyona/security/blob/master/src/impl/java/org/wyona/security/impl/yarep/YarepUserManager.java#L83

or

b)
https://github.com/wyona/security/blob/master/src/impl/java/org/wyona/security/impl/yarep/YarepUserManager.java#L86

I will analyse this issue from now on until I found it, as it is a very critical issue for our productive system. Updates will follow. Feedback is welcome.

Cheers, Bas

[baszero]

UPDATE, SOLUTION:

Root Cause is in the YarepUser constructor where the BCrypt passwords get calculated.

Modify the constructor:

public YarepUser(UserManager userManager, GroupManager groupManager, Node node)

I just removed this section from the constructor method:

if(hashingAlgorithm != null && !hashingAlgorithm.startsWith("bcrypt")) {
  upgradeDoubleHash(this.hashedPassword, this.hashingAlgorithm);
}

and inserted it into a new overriding method:

@Override
public void save() throws AccessManagementException {
    try {
        // Check if we need to upgrade the password hash
        if(hashingAlgorithm != null && !hashingAlgorithm.startsWith("bcrypt")) {
            upgradeDoubleHash(this.hashedPassword, this.hashingAlgorithm);
        }
    } catch (Exception e) {
    }
    super.save();
}

This makes sure that bcrypt hashing only happens when user object is saved. If you just want to read a user object for read-only purposes, you save a lot of time! Actually it's 50 times faster now with this little fix!