even more new firefox reviewers

I’m pleased to announce two additions to the list of Firefox reviewers:

  • Florian Quèze has recently joined the Firefox team, and we’re very happy to have him. Florian has a long history with Mozilla, including a Google Summer of Code working on Firefox Page Info, an internship working on Firefox 3, experience founding and leading the Instantbird project, and most recently work on Talkilla. He’s now helping Felipe and others with work on building translation feature for Firefox, and his extensive Mozilla knowledge will serve him well in reviewing Firefox patches.
  • Mike de Boer joined the Firefox team over a year ago now, and has largely been focused on helping the team ship Australis. He’s been reviewing plenty of patches on the team’s epic journey towards shipping the new look and customization system, and the fact that I’m only adding him to the reviewers list now is a failure on my part!

Please join me in congratulating Mike and Florian!

changes to the list of Firefox reviewers

I’ve made a few updates to the list of Firefox reviewers:

  • Drew Willcoxon, Matthew Noorenberghe, Gijs Kruitbosch, Paolo Amadini, Mike Conley, and Mark Hammond are new additions. They’ve all been long-time contributors to Firefox, and have plenty of experience reviewing Firefox code – their addition to this list is overdue.
  • Mike Shaver and Dietrich Ayala have moved to “reviewers emeritus” status. They’re no longer actively involved in day-to-day Firefox development, so it’s somewhat misleading to new contributors to include them in that list of potential reviewers. Their innumerable contributions to Firefox and to Mozilla won’t be forgotten, though!

Please join me in welcoming the new reviewers – send patch review requests their way! :)

measuring Firefox responsiveness improvements with jydoop

One of the other cool things that came out of the Firefox Desktop work week that I mentioned in my previous post was increased exposure to a handy tool written by taras and bsmedberg: jydoop. Benjamin has a post explaining what it is and how it works, but I wanted to describe one specific use: measuring main-thread responsiveness issues in the wild.

First, a bit of backstory!

One of my team’s overarching goals for a long time has been eliminating all sources of main-thread file I/O.[1] One of the worst main-thread I/O offenders in Firefox code is our SQLite Storage API. A few years ago we went a bit overboard using a version of this API that is synchronous, and does its file I/O work on the main thread. So recently we’ve been focused a lot on eliminating all users of that API.

One consumer of this API that we fixed in Firefox 23 is the Form History code (responsible for remembering information entered into web forms and offering to re-fill it). Neil Deakin, markh and mak worked together to fix the core service (based on earlier work from a previous Mozilla intern, Felix), but after landing that improvement, taras noticed that there were still some remaining issues. We eventually determined that the likely primary culprit for the remaining Form History main-thread I/O was Firefox Sync’s use of the older Form History API. markh and rnewman worked together to fix Sync, but we weren’t entirely sure whether that was sufficient to eliminate all of the main thread I/O we were seeing reported by telemetry.

Now, to the jydoop part

Taras had identified that our initial Form History fix wasn’t complete using the SlowSQL telemetry dashboard. This gives a high-level overview of slow SQL statements occurring in the wild, and he noticed that after we landed the original fix for form history, some formhistory.sqlite statements were still showing up. Unfortunately, the SlowSQL dashboard only offers two time ranges: previous week, and the previous month, and it aggregates data across all builds from a specific channel (e.g. “nightly”), which means that I couldn’t easily use it to confirm whether our Sync fix was effective.

That’s where jydoop came into the picture. It allows much more sophisticated analysis of the SlowSQL data. I wrote a couple of jydoop scripts: one to summarize the slowSQL reports for formhistory.sqlite from builds before the Sync fix (i.e. Nightly builds with a build ID < 20130623) and another to do the same for builds containing the fix. I then ran those scripts against Telemetry reports from the period from 2013-06-21 to 2013-06-24 (inclusive, i.e. roughly two days on either side of the landing of the fix).

The results!

The results are pretty conclusive: the reports of slow SQL statements for formhistory.sqlite from builds including the Sync fix were dramatically reduced. The most frequently slow statement from builds before the fix (“COMMIT TRANSACTION”) was responsible for a total of 417 seconds of main-thread unresponsiveness (174993 occurences of slowness at an average of 143ms of “jank” at a time), over that period. The Sync fix prevented that statement (and many other slow statements) from occurring at all, thus removing a significant cause of main-thread responsiveness issues experienced by Firefox users in the wild.

Next steps

If you look at the results in more detail, they reveal that there are still some slow SQL main-thread operations happening on formhistory.sqlite – they’re less frequent and severe, and are tracked by other bugs. I’m going to be using jydoop to gather more data on the improvements my team’s already made, as well as gauge the potential for improvement in areas we’re still working on. I also want others on the team to learn to use jydoop, so that people can make their own measurements to validate the improvements they’re working on.

[1] Main-thread file I/O is a term used to describe trying to read from or write to the disk on the same thread (the “main thread”) as you’re trying to respond to user input (e.g. mouse/key events) and do other things like drawing the UI. Reading from or writing to the disk can take a relatively long time, particularly if you have a slow hard drive, so we want to always do it on a background thread so that those other important responsiveness tasks (responding to input and updating the UI) aren’t affected.

high-level introduction to Firefox front-end code

At the end of June the Firefox desktop team held a Performance-themed work week Toronto, which included representatives from the Performance team as well as some other teams (QA, automation/testing, project management, etc.). During the week a bunch of us got together and held a little “intro to Firefox code” session – partially for the benefit of a new hire on our team (Steven MacLeod), but also partially because it can be fun to share different perspectives on how we understand Firefox. This was a group discussion, but a lot of it involved me giving my perspective on how Firefox front-end code works at a high level, and how it all fits together. We also went off on a few tangents discussing Mozilla development in general.

mconley was kind enough to take some notes from the session on an etherpad (UPDATE: Etherpad is having issues, here’s an alternate link). These could serve as a good base for an MDN article, or perhaps something similar to https://wiki.mozilla.org/Gecko:Overview.

announcing the firefox-dev mailing list

For several years now, the Firefox project’s official development discussion forum has been mozilla.dev.apps.firefox. It has served us well, but I don’t really ever recall it being an optimally useful tool for development discussions.

I think there are a few reasons for this. I think the primary reason is that the list does not have a moderation policy. We don’t have one partially because it is hard to do given the tools we are using (a custom mash-up of NNTP, Google Groups and a mailman mailing list), but also partially because Mozilla discussion forums have just not traditionally had them. Given the size and diversity of the Firefox user base and community, no moderation means that inevitably there are posts from users seeking support, abusive posts from people upset with our decisions, or off-topic/bike shed threads that dragged on forever and didn’t end up anywhere useful. The combination of these results in a mailing list with a low signal-to-noise ratio, that many core Firefox contributors either don’t read at all, don’t follow closely, or need to filter aggressively. That’s a poor situation to be in, and often means that some of our development discussion ends up occurring in other less accessible, less reference-able venues (e.g. on IRC, or in video meetings).

Three years ago, the Thunderbird folks saw many of these same issues with their development forum, and came up with a solution in the form of tb-planning. That list and its policies have succeeded in creating a more productive and useful venue for discussion. So I’d like to try the same experiment for Firefox development.

firefox-dev is a new list whose goal is to offer an easy, transparent venue for getting constructive, Firefox-related development work done. Some members of the Firefox development team have already started using the list to that end. Its moderation policy has been shamelessly stolen from tb-planning. For the moment, it will co-exist with mozilla.dev.apps.firefox. It’s a mailing list only (no NNTP mirroring update: there’s an NNTP read-only mirror provided by gmane), but the archives are also available (read-only) via Google Groups.

If you’re interested in following or contributing to our development discussions, please subscribe to the new list!

Jared Wein: Firefox reviewer

Planet Mozilla readers are undoubtedly familiar with Jared Wein (a.k.a. jaws) – he joined the Firefox team in June 2011, and has been blogging actively about his work since then. He’s worked on HTML5 video controls, helped mentor and coordinate work on in-content prefs in Firefox, created the Cheevos add-on, been involved with diagnosing and fixing all sorts of Snappy bugs, and many more things that I’m sure I’m forgetting now.

Most recently, Jared’s been spending much of this time helping Felipe, Mark, Shane, and I get Firefox’s Social API ready for Firefox 17. This took a massive amount of work, and while we’re not still not done, we really couldn’t have gotten to where we are without his help.

Jared has been unofficially reviewing code for a while now, and has definitely proven that he’s collected the knowledge and experience required to review Firefox code. So it pleases me to announce that Jared is now officially a Firefox reviewer. Congratulations, Jared!

code review lightning talk

Last week fx-team got together in Toronto to talk about what work we’ve done, the work we have planned for the rest of the year, and to kick start some work on Kilimanjaro-related bugs. We had a round of lightning talks on Wednesday, and I gave one entitled “you should review code”. It’s a little bit audience specific – the call to action at the end of the talk is targeted to core Firefox contributors, but the earlier points about effective code review is relevant to anyone who reviews code, even outside of Mozilla. I posted my slides from the talk, as well as a video (WebM, also available on vid.ly).

new Firefox reviewers

I’ve made some additions to the list of Firefox reviewers:

  • Neil Deakin, a long-time Mozilla contributor who knows XUL inside and out, and has plenty of experience writing and reviewing Mozilla code
  • Blair McBride, owner of the add-ons manager, and an existing toolkit reviewer
  • Felipe Gomes, a jack-of-all-trades who knows browser code well from his work on touch UI, e10s, and other platform integration bits

These guys have been living by and helping promote the Firefox review guidelines for a long time already, so it’s about time that I get around to recognizing that officially. You should ask them for review next time you’re patching Firefox code!

Discussing offensive blog posts

  • “I disagree with the idea promoted by the post, and it upset me, made me feel very uncomfortable and unwelcome”
  • “The post encouraged people to support a law that prevents gay people from using the word ‘marriage’”

 

  • “The post is hate speech”
  • “The post denies the humanity of a marginalized minority group”

For some people, these statements are roughly equivalent. For others, the distinction is quite obvious. I think the degree to which the post offends you affects your ability to make the distinction. I don’t think any of these perspectives is inherently more legitimate than the other – it’s just a matter of perception.

I think the distinction isn’t very important, particularly when compared to the larger issue of a post making a significant group of people feel uncomfortable. But that doesn’t change the fact that a good number of people (particularly those who were not as offended by the post) will be bothered by the conflation, will perceive the second set of statements to be hyperbole, and will be compelled to point them out as such as part of a debate. I think the people who do this do it not because they want to marginalize the other valid concerns about the post itself, but because to them, the hyperbole is just as (or nearly as) offensive as those other concerns. Again a matter of perception.

I think that it’s important, when trying to make a point, to be thoughtful about which variants of these kinds of statements you use. I think both sets of statements identify problems that most reasonable people would agree need to be addressed (and might even agree about how to address them!). But the set that can be perceived as hyperbole is less effective at creating empathy and shared understanding, particularly in a very tech-y community like ours, because the distraction of semantics leads the debate into unproductive (and secondary) areas.

Firefox module changes

Shaver posted about this in dev.apps.firefox last week, but I thought it might bear repeating to the Planet audience: as of last week, the Firefox module has a new module owner (yours truly), and a new review policy.

The new review policy is explained in detail on the Wiki. As part of this change, there is now a larger set of people deemed “Firefox reviewers“, and a new policy for nominating reviewers. Together these changes will hopefully address the review bandwidth issues we’ve been having.

Because the Firefox module is relatively large and covers many different code areas, Firefox reviewers are expected to know their areas of expertise, and not be shy about redirecting requests to a more suitable reviewer, if applicable. The wiki page also goes on in detail about what r+ means in the Firefox module – a handy list to review for new and old reviewers alike.

Please, send me email if anything on that page isn’t clear, or if you think anything (or anyone!) is missing!