Rewriting and mork

The Great Address Book Rewrite is progressing along. Now that cardForEmail and cardForProperty are both done (note: I am including as "done" what is merely awaiting r/sr), casual users of nsIAbMDBDirectory and nsIAddrDatabase can be refactored to use generic directories.

The directory one is completely eradicated (outside its valid uses), excluding the not-as-invalid use of being used for the database and its use as an include for some constants, which I'm not sure is still valid. Databases, on the other hand, are still very much alive (I intend to kill off the one extant in nsMsgCompose shortly).

Databases are used, of course, in the MDB directory stuff. But LDAP replicates only to a database. That's probably not hard to fix. Palmsync uses the database quite liberally. That's annoying because palmsync doesn't compile by default, I don't use it enough to be able to test for regressions (I have a PocketPC, not a Palm), and I develop primarily on Linux, so my best shot is most likely to take a go at cross-compiling it.

Last but not least is the granddaddy of database usage, import. It in fact uses nsIAddrDatabase more often than addrbook itself, and that statistic is lopsided since nsLDIFService in addrbook is the LDIF import, and addrbook includes it for constants a lot and also defines the interfaces. And import consists of four different importing address books, all of which have to be changed, and at the same time. My previous aborted attempt to eradicate the database from nsLDIFService alone took hours, required careful removal of hacks, and was probably not tested. The kill-database-in-import patch will likely find itself over 100 KiB.

In any case, I now believe that the setup is sufficiently abstracted to permit me to start work on an SQL-backed address book to replace mork, with the goal of finishing it by TB 3.0.

Mork and SQLite

As I was doing some performance testing for a bit of demorkification, I noted that what I was demorkifying was one of the few places where mork works well. The API makes it very clearly an EAV model, which is a model that makes mork happy. On a higher level, it would be hard to position it in a way where the queries only affect one row (or column of a row in mork).

Mork is a model likes EAV and hates queries. That means keys need to be meaningful, since that's the only query you can do without looking at everything. Meaningful keys create problems, though.

SQLite is a model that loves queries but hates EAV and other models which don't rely on batching. That means that to get good performance, you should really be using larger batch queries. The biggest implication is that querying suddenly drops down to a lower level of index. If you pick the queries and schemas right, SQL can knock searches down from O(n) to O(lg n), and forgo object creation costs as a benefit.

But back to the patch at hand. Basic testing shows that, optimizing what I have to the point of not changing some core implementation stuff, it regresses badly in relative terms on two infrequent items (creating and removing files, i.e., adding/deleting folders). On actually retrieving the cache elements, it does the same (under <1ms average for both on a release build). Surprisingly, mork hates commits, consistently taking longer without a compression and averaging at 15.36ms for 50 commits, while SQLite takes 2.41ms on average for the same 50 commits. The cost, though, is in get/set of the actual EAV pairs; while all operations are under 35ms for 5000 get/sets for SQLite, mork can do the same for 10ms per operation. But mork's subsequent speed comes at a 100ms startup price (for 50 folders, heavier users will consume even more time).

Mork's fast query comes at the price of a slow startup: it loads everything into memory. SQLite could do the same thing, which would improve the get/set times but weaken startup, as well as memory usage as well. Mork also makes committing—which apparently happens more often than actually setting a property—more expensive, especially if you've neglected to compress recently; SQLite shows no loss if you neglect a compression, and (for something this small and static) no strong benefit to vacuuming. I couldn't find any services to measure memory easily, so I'm relying on assumptions there.

A blizzard of updates, part 2

Yesterday ended the second day of blizzard updates. Today I'm attending a hardware setup party, so I won't have a full day of blizzards. What I did do yesterday:

More on nsIAbCard

I now have a patch once again requesting review in this area. nsAbOSXCard drove me mad when I was going back over my changes. Something about aMember.Equals(aValue) didn't seem like it made sense; it turns out that it wasn't quite right, because the meaning of aMember had been changed ever so slightly.

nsIAbDirectory

I started looking into replacing more of nsIAddrDatabase in import, and immediately backed off. Crazy stuff happens there, so I'll need a few hours to work on that without distraction. I've also hooked up nsIAbCollection and am slowly starting to make it work right. Keyword: slowly.

nsNNTPProtocol

Getting the list of newsgroup is somewhat confusing, as it launches some callbacks to avoid hanging the UI thread. Life will be so much better when protocols go async.

libmime

I didn't spend as much time on this as I expected to, but the initial forays look promising. Its implementation is amazingly simple for having a custom C++-like format, so an automated rewrite should go smoothly. A naming scheme is rigidly enforced for the class/object hierarchy, and connecting the function implementations to the defined virtual function pointers looks trivial. Finding the inheritance list is simple, especially because it doesn't practice MI. The hardest part, though, is constructors. They may need to be done manually.

MorkReader

My biggest announcement is the one I'm saving 'till the end. I've started work on MorkReader.cpp, after spending a few hours going through the morkThumb morkParser morkBuilder code to see how the most complete implementation of mork sees a file. Did you know that db/mork doesn't actually fully implement the mork specification? Also, groups.google.com appears to have spotty records of old netscape.mozilla.public.mail-news postings (I need the ones from late 1998/early 1999), and news.mozilla.org only goes back to 2003. Fun times...

And for today? A hardware party, followed by some more work on MorkReader. Hopefully with ample help from David Bienvenu.

Mork is evil, but...

I decided today to finally start poking around the Mork reader code that was introduced to import history data, since the plan is to use it to migrate mork to SQLite code. I had read earlier enough to know that I would have to look into having morkreader handle multiple tables, but what I saw was just astounding. You see, morkreader is essentially a 580-line hack.

Don't get me wrong, I have nothing against hacks. My patch for fixing searching in base64-encoded messages was quite hacky as well. But a few things justify the hack. First, the legacy code needed some pretty severe refactoring to handle the recursive nature of MIME. Second, the improperly-handled cases should be rather rare in nature: the simplest way to generate a case is to forward-as-attached a message with a base64-encoded attachment. (I concede: a third reason is that I wrote it, but that pales in importance to the other two, I swear...) Morkreader, however, did not need to work around crufty legacy APIs, nor are its improperly-handled components uncommon.

The first assumption that morkreader makes is that there is only one table (mailnews loves having several tables). Adding in support for multiple tables would not be too difficult if the parser wasn't already broken in other ways. The number two assumption it makes is that no line is longer than 80-characters (which should be safe) and that no line is continued more than once (i.e., no more than 160-characters)... which is complete BS, as anyone who has ever subscribed to a mailing list can recognize (think of the References: header). Finally, the code will not handle aborted changesets properly, the banality of which I cannot determine. (Does mailnews code ever use the ! change type?)

So, to assess the accuracy of morkreader, I read the mork specification. Cross-referencing with some mork files of mine (a FF 2 history.dat, an abook.mab, and an inbox msf file), I discovered that the specification itself is inaccurate. Once again, two failings here: (atomScope=c) should be (a=c), and the spec implies that -[...] is the proper way to remove a row, whereas [-...] is the actual method. A subtle statement in the spec says that a + can be omitted from changesets.

I had to fix mork to get my first patch for bug 413260 working, looks like I'll have to fix morkreader as well. Oh well, magic will happen if Friday is as predicted...