Thursday, August 7, 2008

The Great Address Book Rewrite

The nsIAbCard portion of the Great Address book rewrite landed not long ago. This patch represents the first major overhaul of the address book interfaces in a while. Most specifically, it removes the nsIAbMDBCard interface, adds the nsIAbItem interface, and completely overhauls the nsIAbCard interface from being a flat list of properties to essentially a spruced-up hashtable.

These changes, 295.33 KiB of them for a -U 3 patch, comprise a removal of 2,732 lines of code for 1,871 lines in a total of 52 files, some of which are OS-specific, and the culmination of over 6 months of editing and revising, a timespan also including a change of computers. I've changed the internals of nsAbCardProperty at least three times, and fixed two bugs in a heretofore unused mork class. The APIs I worked off of have changed several times, partially as a result of my experiences with the work, and partially otherwise. Finally, I have my chance to bitrot other people's WIPs instead of finding that any addressbook change bitrots the patch I have (or another in my queue).

The good news: nsIAbCard is now effectively stable to use in extensions or other core code not yet pushed. The only caveats are these:

  • getProperty* has had some late-breaking API discussions, related to how to handle properties that don't exist. It is possible, but not likely, that semantics of existing methods would be changed.
  • copy is not the best thought-out method and will change in the future.
  • equals has some potential problems, such as the fact that its use in circumstances may violate the reflexive property.
  • UUIDs were stripped from the patch pending future design decisions.
  • The two pre-existing properties will exist only as long as mailing lists are insane.

Will this patch create regressions? Probably one or two minor ones; changing an entire internal model, a heavily-used API, and implementations across several files, some of which are OS-specific, and others which are app-specific makes a regression likely. But I have taken effort to find problems, combing through the little import/export I can test with Linux, as well as using all the features I could. grep has proved invaluable, as well as a few tests of Dehydra to find callees.

What's next from here? The next logical step is the creation of nsIAbCollection and related refactoring. However, async APIs and error conditions are still in flux for that. Not all of the OS-specific directories are fully squared away with the new model, but I have limited testability in those areas. UI unforking is a place where we could win big, especially as it saves me a few places to look for usages. Import and palmsync may also want some more love. Naturally, there are also the spin-off bugs, but many of those are not immediately fixable.

For me, at least, I now have some time to work on other bugs while I wait. There are 12 other bugs in my queue, ranging from an IMAP fakeserver to an aggravating news bug to a brand new module (O.K., total rewrite of an old module), 6 of which are more or less ready to be committed. Oh well, life goes on...

Tuesday, August 5, 2008

I want a pony #2

Okay, continuing on trying to find obsolete members. One feature that I'm sure everybody would love is a JS static analyzer. And while we're on the topic of JS, how much longer until Mozilla gets ES 4.0 and static typed JS?

Maybe this isn't so much a pony as an entire ranch, though...

I want a pony

As I was walking through nsIMsgIncomingServer in search of unused attributes, I noticed that some attributes were added in a revision and their usages removed later. But the checkin comments don't give me any clear indication of when it was removed and what replaced it. Anyone fancy binary searching hundreds of checkins just to find one property?

A simple solution to this is a reverse annotate feature: a script or similar that, given a revision number, will look through the subsequent revisions and find out which version (if any) first changed a given line. Bonus points for ignoring whitespace changes. I could probably hack out such a feature myself for hg, but I know so little about CVS that it's hopeless for me to try there. And since most of the important history of mailnews is only in CVS, an hg-only becomes pointless, at least for the next, oh, 5 years.

And before anyone complains about me wanting a pony, let me tell you this: my cousin actually got a pony for Christmas last year (after asking about it for the last, oh, 17+ years).

Monday, August 4, 2008

Thanks, dbaron

Earlier today, bz r+'d bug 366791, and dbaron then checked it in. For anyone building TB, this represented about 95% of the assertions and 60+% of the output (being such a long assertion doesn't help) running TB normally. This makes breaking on assertions feasible now, with so few false positives. Now, the only spammy thing left is the "recurring into frame construction" warning.

So, in short:

Thanks dbaron!

Wednesday, July 16, 2008

Profiling made visual

When you've got a performance regression resulting from a major patch, pinpointing where you can save time can be annoying. For me, on Linux, the only decent tools is jprof. And I didn't get far in jprof before tripping over a bug in its code that made reliable testing infeasible (ternary operators are wonderful things). After fixing that, I turned to the output.

The output is basic, and, in general, not helpful for deep inspection. Okay, so I know that I'm spinning in this function in specific. But, on a grandiose level, which functions am I really spinning hard in?

In one trace, it's obvious that malloc, JS, card creation, and case conversion are being nice and expensive. All four of those are more or less unavoidable. Where else am I wasting time? It's hard to tell, since many of the top functions produced by both flat and hierarchial views are wasted by irrelevant subfunctions of these. Enter graphviz.

Graphviz is a wonderful library I discovered about a year ago. It takes a file that looks like the code on the left and makes it into the graph on the right.

digraph G {
  A -> B;
  A -> C;
  B -> D;
  C -> D;
}
Graphviz output

The output gets better as you tickle it more and more. But it's flexibility is not why I love it. It's the fact that the simplicity is such that one can easily just write a simple sed or awk script to generate the graph. In the following three commands (that could just as easily be one command, but I'm not that cruel), I took the ugly jprof output and formatted into an easy-to-read graph:

jcranmer@quetzalcoatl /src/tree2/mozilla $ cat tmp3.html | sed -e '/index/,/<\/pre>/!d' -e '/<A href="#[0-9]*">/s/^.* \(.*[0-9]\) \(.*\)<\/A>$/c|\2|\1/' -e '/<a name/s#^.* \(.*[0-9]\)</a> <b>\(.*\)</b>$#f|\2|\1#' -e 's/<hr>/e|--/' -e '/|/!d' -e 's/|\(.*\)(\(.*\))|/|\1|/' -e 's/|.*::\(.*\)|/|\1|/' | awk -'F|' 'BEGIN { skip = 0; print "digraph G {" } $1 == "c" { if (skip == 0) { count[$2] = $3; } } $1 == "f" { for (func in count) { print "\"" func "\"->\"" $2 "\" [label=" count[func] "];"; delete count[func] } skip = 1; print "\"" $2 "\" [sum=" $3 "];" } $1 == "e" { skip = 0 } END { print "}" }' > ~/full.dot
jcranmer@quetzalcoatl ~ $ cat full.dot | gvpr 'BEG_G { $O = graph($.name, "D") } E  { if ($.tail.sum > 200 && $.tail.sum < 1000) { copy($O, $); } }' > full2.dot
jcranmer@quetzalcoatl ~ $ dot -Tpng -o full2.png full2.dot

Now that I've most likely burned you eyes out by using a sed, an awk, and a gvpr (something like awk, but for graphviz) script all from the command line, I feel the need to explain what it's doing. The sed script, in order, grabs only the hierarchical portion of the jprof output, changes the lines into simple fragments surrounded by pipe characters to be readable better by awk, and then scrubs the C++ demangled names into simple function names (although not perfectly). The awk script then compiles the information into a dot file mapping the call graph and annotating the nodes with probe frequencies. Next, gvpr scrubs out all nodes with more than 1000 probes or less than 200 probes. Finally, dot gets a hold of it, and makes a nice PNG of it.

And the PNG is informative. Although enormous, the information leaps out immediately. Floating high up are five functions which are expensive, the fifth of which I never noticed: XPCThrower::ThrowBadResult. Hmm... I quickly threw up a graph of the pre-patch results, and confirmed that it wasn't in the top slots there. Doing some basic math, this one function, and results off of it, produces about 60% of the current regression, assuming that I'm looking at the numbers right. Who said throwing exceptions was cheap?

Anyways, my visual approach to profiling isn't complete. The graph is in plain black and white, where I should be using colors and line thickness to be representing the expensiveness of operations. I might also play around with tickling the data to be able to highlight exact functions where regressions occur, something that I could easily do with gvpr if I had two dot graphs of the translated output. And my output filtering isn't perfect by any means. But all that comes for free in my envisioned perfect profiling extension. Oh well, at least I have something to point to for neat data.

Thursday, July 10, 2008

2900- UNCO bugs

As of a few minutes ago, Thunderbird's UNCO bug count dipped below 2900: it stands at 2892 right now (likely to change within a few minutes). For those of you not satisfied with only one products bugs, the sum of all mailnews UNCO bugs (TB bugs + mailnews portions of MAS and Core) is 3236, with Core contributing 227 and MAS with the other 117. Keep triaging!

Wednesday, July 9, 2008

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.