Making the New Password Manager Storage Faster

It’s a simple fix, but it makes a difference.

Posted: August 28, 2008 Tagged: mozilla, firefox, javascript, passwordmgr, sqlite, and mozstorage 2 Comments

When I first wrote the new storage module for the Password Manager, I took a few “shortcuts,” trying to keep my code DRY. Partially this was because of the first patch by Mrinal Kant, but mostly it was because I like to reuse code. This bit us just a bit.

I mentioned in my first post about this change that we were initially considerably slower in the critical countLogins method. While it got improved before being checked in, it was still marginally slower (milliseconds on an abnormally large dataset).

As I said before, this was most likely since we were doing a SELECT * on the moz_logins table, and looping over the results and counting. This allowed me to reuse more code. Loops are a kind of slow, and since this was so important, I decided to speed it up.

I filed a bug just over a week ago entitled “storage-mozStorage should use COUNT in countLogins” – which pretty much explains what the solution to the above problem. I created a patch which essentially just switched the mosStorage module to use SELECT COUNT(1). I reran the performance tests I created and we’re doing much better now. There’s still a miniscule loss in speed from the legacy storage module, but at this point, we’ve done all that we can, and where the difference was milliseconds, its closer to millisecond.

This was checked in today (thanks Justin!).

And that’s it. I have another patch in the pipeline and hopefully I’ll have time to get it finished, approved, and reviewed for the freeze (whenever that is now).

Comments

  1. August 28, 2008

    Just to be clear:

    • The version that was “considerably slower” was caught and fixed before the initial checkin of the mozstorage port.
    • The first version that landed was equal-to or faster-than the old code in all cases, with (iirc) the one exception: if the user had many (hundreds) of logins saved for the site being loaded [ie, the target of the countLogins() call]. And even then, it was just a few milliseconds difference.

    It looks like Anonymous freaked out a bit on your last post, so I just wanted to make sure there was no confusion. :)

  2. August 29, 2008
    Author

    Thanks for clearing that up Justin (and of course for all the help you’ve given me throughout this project).

© Copyright 2004–2010 Paul O’Shannessy Powered by Blarg & hosted on Dreamhost