1 of 32

remedies for frustration

with speed, quality, and reviews

Martin Pool

linux.conf.au 2013

(open the speaker notes)

2 of 32

3 of 32

4 of 32

5 of 32

performance

Let's kick off with performance: speed: being fast enough to like the program's working with you and not getting in the way.

bzr was far too slow in its first releases: it started out as an update to GNU Arch and was faster than Arch, but not fast enough in absolute terms. It got a lot faster, but it disappointed or frustrated, and in some cases permanently lost, early users in the mean time.

In very early bzr releases I focussed mostly on the UI behaviour and the functions of the data model behind it. There was an intentional strategy to “make it work, make it correct, make it fast,” especially because we were starting from a system that worked but which had an unlikeable UI.

The UI and user model was one that many people have liked, but deciding to optimize later turned out to be a problem, and it took us a while to get away from that.

"Premature optimization" is a dangerous concept. It tends to imply you can defer all consideration of performance until you have the behaviour and UI you want.

But, it turns out if the UI, core APIs or formats are hard or impossible to implement quickly, it is hard to unwind them.

6 of 32

<jelmer> rewriting is often a good idea,

it just never works out

7 of 32

design for scaling

Now I’d have a more nuanced view of what “avoid premature optimization” means: something more like “avoid premature micro-optimization.”

The key question is: how expensive will it be to come back and change to a faster implementation later on, and how much does it save you to write a simple or naive one now.

If there’s one function or routine that is written in a straightforward way, going back to rewrite it should cost only whatever time it takes you to rewrite it, and then to test, debug and release that change. So it will often make sense to defer that work until you’re sure the requirements for the code really satisfy the user, your understanding of the problem is correct, there are no unknown bugs or edge cases, etc.

However, there are some things that can make changing the code much harder and we ran into several of them:

1 The user model or user interface requires something that is inherently expensive to calculate: for example, that revisions are numbered by counting across the whole revision DAG from the origin, an operation that is O(N_revisions) or worse. Changing the algorithm may require changing the user interface. Or, you may be able to mitigate the performance impact by adding a cache (but that has other problems, see later.)

2 If internal APIs embody a slow approach, improving it may require reworking the APIs and that may require changing a lot of code, possibly out-of-tree code. For instance, many Launchpad APIs were structured as a URL per object, which could be read or written. Quite straightforward: but also painfully slow if you want to update a hundred bugs, or search through a distribution with hundreds of thousands of bugs. (The antipattern has been called: "potato programming: one potato, two potato, three potato, four...")

3 If data formats are inherently slow, changing them may require migrating the data, which means, perhaps: conversion code that must remain in the tree for a long time; incompatibility with previous releases; explicit user action and decision to upgrade; risk of bugs in the conversion code.

So what we should have said, (and later did say) rather than punting on performance altogether, is to explicitly add a requirement that for example detecting there are no changes completes in under 1s on a tree of 10,000 files with 100,000 revisions. We might not have hit it in the very first release, but it might have helped us avoid doing anything that would make it harder.

(Tangentially, this can also be done very well for robustness: for example, one might set a requirement for a backup system that even if one file is corrupted, everything else can be restored.)

One lesson is to assure yourself, before settling on a format, API or UI, that it is at least possible to write a fast implementation. Of course actually writing it is very good proof, but short of that it may be possible to at least make a convincing argument that the specified behaviour only has to access a limited amount of data.

I think it's also good advice to do this planning while considering 10-100x the expected input: if you expect to deal with 1GB files, make sure the basic design can handle 10-100GB files, which probably means not assuming they will fit in memory. This will give you a margin of error for the implementation, and hopefully give the approach some longevity.

8 of 32

deterministic effort tests

Truly was it said that "untested code is broken code", and if the performance aspect of code isn't tested it's likely to regress too.

There's no substitute for testing real-world performance. (I wrote a little program, 'judge', that runs two programs and tells you whether there's a statistically significant difference between them.) And there are other tools that will let you track test suite run time over time.

But the problem is, real-world measurements of performance have a lot of noise in them: contention against other processes on the same machine, varying filesystem performance, things being in cache or not. And if you want to measure network speed, there's even more uncontrollable variation. And the numbers can't easily be compared across machines.

This is a bit of a problem because we'd really much rather always keep the tree green and flag things that can cause performance problems before they get in - ideally the test suite would fail if and only if you had introduced a performance problem.

Our solution was what I call 'deterministic effort tests' - we count operations that will correlate with performance and where the number of operations should be deterministic for any program and input data. For instance the time it takes to do a network request may vary widely from one test run to another, but the number of requests might be constant for a particular function, and adding more of them shouldn't be done accidentally.

One subtlety: you might want to assert that "there are no more than ten requests", but it's actually much better to say "there are exactly ten requests." If someone improves things so that there are now only eight, the test will fail - but then they can update the test, and you will catch any accidental regression back up to ten. (It's complicated.)

Memory use is also important but noisy, so hard to deterministically test. In C/C++ you can account for every allocation. In gc'd languages it's a bit harder. One technique is to impose a budget: to say, this program should use constant memory regardless of the input size, or it should use a constant plus one copy of the input, but not two copies. Then set a ulimit on the program, run it on a large input, and see if it fails. If it does run out of memory, it's highly likely to have failed on the call stack that's responsible for most of the memory allocation.

And another variation on this: set a deadline on server request time; flag requests that exceed the deadline. Use this as your performance metric, not the mean or 99% request time, which will be noisy.

9 of 32

performance test tips

tc qdisc to locally simulate a predictable slow network.

/proc/sys/vm/drop_caches to reduce vm variation.

10 of 32

fast Python

We tried to have a fast pure-Python implementation, so it would be easy to install anywhere, and easy for anyone to work on.

Of course the key thing is (as discussed elsewhere) to have the right algorithms, ui, and data formats so that it is possible to write a fast implementation - so that you are not doing work that could simply be avoided.

But even if you are doing only the minimal work, you still want the scaling factors to be low.

It turns out, some things in Python are expensive - a function call is on the order of a microsecond; user-defined objects are also fairly expensive to construct. So there is a pressure to store everything in lists and tuples, and to inline code as much as possible. But code written in this way becomes pretty hard to maintain, and you're giving up a lot of the benefits of using a high-level language in the first place.

So my advice now, would be: just don't. Get the algorithms right, and write Python that's straightforward and not inefficient. If that's not fast enough, rewrite to Cython, Pyrex or C, rather than writing convoluted Python.

11 of 32

be careful of caches

12 of 32

quality

13 of 32

tested fakes

Key subsystems provide their own fake implementation.

Use testscenarios to run the same tests against all implementations.

Some things are hard to test: slow, unreliable, not always available. For instance

So it’s common practice to use Mock or Fake objects

But the Mocks can drift from reality, which is dangerous. Many Mock libraries encourage you (at least in their examples) to declare the mocks inline in the code.

The key thing is that they should be defined next to the class they imitate, not in the test that uses them.

Fake objects are one totemic case where we want to make sure that two or more classes provide similar functionality. Another is where there is an abstract interface, such as a Transport, with multiple back-end implementations: local files, ftp, sftp, bzr-over-ssh, etc.

And, there should be tests that check they behave the same as the real class, in all but the dimensions where they are intended to vary. Use test scenarios for this. Express a set of requirements for everything that provides a given interface, and at test time generate one test execution for each scenario.

14 of 32

Done Done

A feature's not really done when you commit the code for it. Or when you merge it to mainline.

Lean systems thinking encourages the useful definition: the work is done when it's available to a user, and it works for them.

That means: it's merged. It's in a release (if your software makes releases), and the release has been packaged or built, and it has been through whatever quality or release processes are necessary that people will automatically upgrade to it. If it's a web service, "done done" means that the code is live on the production system. If it's a package that people receive from their OS, it may mean it's not done until your bug fix has been pushed into their stable update channel.

It can be easy to lose track of things once they're merged to trunk, but they're not done.

And, "done done" means, too: it actually works! If you release it but it's buggy, your users aren't happy with it, you can't move on to something else, and they're not getting the value of your work. So, in fact, being "done done" may take longer than the first release that includes the feature, until you get any necessary feedback, integrate it, and make a new release.

The definition of "buggy vs working" can be a bit hard to precisely define. I'm not saying you can't count something as done until it's utterly perfect and no improvement is conceivable. The balance is: it's working enough that the work was worthwhile: all the major features put in are sufficiently reliable. And, of course, it's fast enough. Ultimately, users decide what's good enough, and therefore when the work is done.

15 of 32

A ramp for testing

One cause for latency between work being done and being generally available is:

- there are delays for "testing" or "soaking"

Over the lifecycle of a patch, it's gradually exposed to an increasing number of people, starting with just the one person who wrote it, ramping up to your entire user base. There will be bugs. There is value catching them earlier rather than later: fewer people suffer the consequences of the bug; perhaps you spend less time on support; perhaps your reputation is better.

However, there are some bugs that require wide exposure to surface: they only occur in some environments; they occur or are noticed rarely; they occur only when the product is used in an obscure way; they are more obvious to some users.

We can think of it as a pyramid, gradually accumulating more users. We want it to be short (in time) but not too short. Always be asking, whether each of the release phases you have could be made shorter and still catch an adequate number of bugs. Perhaps if you can release faster, it will be cheaper to recover from a bug, and you can tolerate slightly more of them getting through. Perhaps for a web service you can make it easy and safe to roll back, or you can expose features at first to just a subset of users.

Perhaps you can have a "stable series" that gets fewer updates, and then be just slightly more risky on the main series.

There is a real cost to protracted testing; the question is not "have we found all the bugs" but "is this test phase long enough it's reaching diminishing returns"?

16 of 32

high-speed delivery

<jml> coding leads to deployment; deployment leads to waiting; waiting leads to anger; anger leads to ...

If we look at our work as being a cost, but not a benefit, until it's delivered and accepted by users, then we should look to make that interval as short as possible.

You may think, well, there are many features outstanding, but they're all getting delivered eventually, why does it matter how long any individual one takes? Why are you concerned with latency, not throughput?

It's a good question, but there are several good answers.

One is argument is the time value of money: you get benefits, of some kind, from shipping better software: more users or contributors, fame, money. The sooner you get your good work out, the faster those benefits can compound. A thought experiment: zero releases for five years, and then one big lump: but for those five years nobody else will have contributed to, or even used, your project.

By the time it arrives, it may no longer be wanted. The user for whom you were fixing the bug might have given up and left.

Another reason is that unfinished code can rot. It doesn't get fully tested, all the way through to testing by real users. It may conflict with other work, or they may get tangled up in dependencies. With everything in a continuously-releasable head, things are simpler.

17 of 32

look for latency before work is

- reviewed

- merged to mainline

- into a release

- built into binaries

- on the production server

- in the OS or auto-update channel

- supported by all related systems

- discovered by users

- working properly...

I want to suggest you should look for cases like these (and I'm sure there are more) where you can have done some work, but your users are not yet benefiting. Systematically observe them and try to change your processes to eliminate them.

They can be well hidden and it can seem impossible to change them, but you can change them. Typical causes:

- people "get credit" (with their manager, or in their own mind) for work that's not fully done

- some of the work is not automated, therefore people do it irregularly or unreliably, or the processes seem to expensive to do them frequently

- no one person can do every stage: so there are delays for handoffs, or different people are busy with different things

Avoid starting new work if you could instead be finishing delivery of existing work.

Parallelism is an adaption to some problem preventing the original task being completed immediately: treat it as a sign something may be wrong.

The longer the work is outstanding: not only have you paid the cost and not got the benefit, but also: it may never get completed, or it may turn out your concept was wrong. There is a a real "time value of money" thing here.

Look also for an insidious form: planning work far in advance of it getting done. There's nothing wrong with thinking ahead. But, be skeptical of the balance between planning future features in detail, when you should be finishing off work that's already been started. Are the future plans going to affect or constrain how you work today?

18 of 32

ordering ≫ scheduling

Estimating when software work will be complete is (A) expensive and, (B) impossible. So, I suggest you should avoid it if at all possible.

I say impossible because there is an extensive literature that shows for new creative work, people are both imprecise (large multipliers of relative error) and inaccurate (almost always undershooting.) This is especially so when you include the whole time to get the feature done done: deployed, debugged, and accepted by users as a good thing.

One place you may need estimates is to decide which of several courses will be most worthwhile. It may be worth digging in to the alternatives to compute their relative ordering. Another place is where there is inflexible external coordination - launch parties, hardware manufacturing - that needs a firm date.

Rather than spending time predicting which features will be in which release I suggest you instead:

- get a clear focus on which is the most valuable feature to do next

- remove delays between finishing that feature and making it available for use

19 of 32

bug management

20 of 32

bugs: now, soon, never

The book Getting Things Done makes a very algorithmic observation about the way people organize the work they have to do: as much as possible you want to look at each incoming request or task only once, and make a decision about what to do about it. Looking at the messages in your inbox over and over again to pick one out is O(n^2) - not very efficient!

Similarly for bugs: we want to avoid spending time repeating the same assessment of the same bugs.

There are only three real priorities for bugs (including enhancements etc): now, soon, never. "Now" are the really critical ones, so important that you will drop other work-in-progress to deal with them. "Soon" are the ones that have a good chance of being taken when you're next idle: the size of this set is approximately the number of bugs that your team completes in two or three months.

Anything that is not within those inner sets is highly likely to never get finished.

You might think: well, no. We'll do all the Now and Soon bugs, and then we can get in to the lower-priority bugs. But generally, that's very unlikely to happen, because this is a dynamic equilibrium. Whatever process caused the Now and Soon queues to fill to their current level is likely to refill them before you ever get to the Never bugs.

You can measure the expected time-to-fix of your current priorities, and work out where these map.

If the Never bugs will (mostly) never get fixed, by the Lean concept above, any time spent on them is a waste. That includes code written towards them that won't get landed, also investigation and triage. You should be uncomfortably brutal about ignoring unimportant bugs.

Now this is a bit oversimplified, perhaps. It may be that what looks like an unimportant bug is in fact a clue to something very seriously wrong. It may be that a really low-impact bug is still annoying so many people that it ought to be upgraded, or that it just happens to irk a developer. Bugs can be upgraded - it's Limbo, not Gehenna. But we don't want to do any work proportional to the number of bugs in that unbounded bucket - things should need to specifically surface to get any attention at all.

I don't think even trying to measure that set is useful - for any nontrivial program, the more you look, the more bugs you will find. You might as well just assume it's unbounded, and use some other measure of your quality - such as, how much time you have to spend fixing bugs vs making forward progress, or how long it takes for a new feature to be stable.

Of course, if someone does feel a bug is important to them and they choose to write a patch, they ought to be helped.

There is also some value in will-never-be-fixed bugs to users: they can know it's a real bug in the software, not their fault; they might find a workaround; I'm not opposed to them being tracked, only to developers spending much time gardening them. It's busy work: it feels like you're getting something done but you're not actually producing any value.

Some of the Launchpad bug tracker and the Ubuntu crash tracker help of this, but I would like to see much more done in open source bug trackers towards this:

- default towards doing nothing

- for 'never' bugs (the majority), developers should do either very little or zero work

- bugs should decay from view

- still make it possible to dig out old bugs

- good automatic dupe-finding, including retrospectively or interactively hunting for dupes

- show an automatic estimate of the odds a bug will be fixed

21 of 32

black and white bugs

Another aspect in which we can avoid repeatedly making the same decision about bugs is to make sure all bugs are written such that you can make a clear decision.

Bug trackers tend work well with discrete classical bugs: “when I type ‘foo’ it crashes”, or “when I type 2+2 it prints 5.” These are good because it’s fairly easy to say whether the bug is still present or not, and moderately easy (for someone who knows the program) to say whether two reports are instances of the same bug.

They have much more trouble with: feature requests, UI shortcomings, and performance bugs. If there’s a bug that “commit is slow”, how do you know when it should be closed? How fast is fast enough?

One experiment was to put a semi-arbitrary goal in the bug description, but that had limited success. One more useful thing was analogous to what we did with effort tests: investigate the slowness enough that

One problem I’ve seen in other open projects, like Ubuntu, but not so much in Bazaar, is that some bugs that are matters of project policy, can become more like a petition or protest by the users against a course they disagree with. There’s nothing wrong with people having those opinions and the project may benefit from hearing them, but doing it in the bugtracker just seems to cause noise and contention...

22 of 32

community

23 of 32

a model of developer growth

How do developers come to an open source project?

Perhaps some have decided (especially for famous or commercially-backed projects) that they just want to be a Foo developer, but more often they are users first, and they have an itch to scratch.

Before anyone was a core developer, they sent a first patch. If they don't write a first patch, they will not write a second, a third, or a fourth.

We could ask, how do people decide to attempt to change the software they're using? And probably, there are many interesting questions there about how they discover such a thing is even possible, and then whether or how they work out how to make a change.

Mentioning that, makes it very clear there is a funnel, or a series of ever-narrowing gates. Not everyone who learns of your software will download it. Not every one of them will run it. Not all of them attempt to change it, not all of those succeed, not all of those who succeed in changing it will send you the patch. Not everyone who sends you a patch will have it accepted; not all those who have it accepted will attempt a second patch.

There is, unavoidably, some loss at every stage. Not everyone wants to write open source software; not everyone has any relevant skills; not everybody has the time in their life to do everything they might want to.

But I think some of this is avoidable: you can shift the balance so that it's more likely people who see a bug and are able to fix it will actually fix it. You can improve the odds that people who experience a bug send you a patch, and you can improve the odds that having sent you one patch they will come back with another.

And far along this tortuous path, we can say people have become a core developer. They can commit to trunk; they do code reviews for others; they actively share ideas about how the project can develop.

24 of 32

a bad code review

25 of 32

the purpose of review

to ensure people do not add trailing whitespace?

Code review is good. But why do it? What is the purpose?

We should ask: what do we really want from our project? Many of the things that tend to come up during reviews, like fine-grained formatting, are not necessarily what people would most value in their project. Is your highest aspiration really to have everything style-guide compliant? I suggest it is not: we want to make something that our users love, that attracts the best work from good developers, that stays maintainable, and that is fun to work on.

What would the review process look like if we really focussed on valuing these things?

Some bugs are detected in review, and it is worth looking for them, but not all bugs will be apparent in review. (As before: question how whether a quality-oriented process is giving good value, compared to proceeding to the next stage of testing.)

I think:

- detect bugs

- enforce coding standards

- keep people up to date about progress of the code (at least one other person has to read it)

- ensure test coverage

- ensure design consistency

- help people think of different ways to write something

Now, new developers are most likely to be fixing fairly shallow bugs or adding fairly small new features. They're unlikely to be making large rearrangements of core designs.

They may well have bugs and they may not know what kind of bugs they can cause. They may not entirely know what facilities are already there to achieve what they want.

It's the more experienced developers who, by definition, are doing the largest work and who are probably the most prolific. So for new developers, even if they don't write things in exactly the ideal way, I suggest to you that doesn't matter so much, because the code they're changing is probably fairly limited in extent and straightforward in function. Similarly: they should have test coverage, but it doesn't necessarily need to be tested in the very best way: an integration test may be ok, and may be much easier to write, even if generally you would prefer to have tests at several levels.

So I want to suggest we should have somewhat different weightings on these standards. For new developers:

- no obvious bugs

- some test coverage, perhaps just in the most obvious way

- written in basically the right way, but not strictly minimal

- not violating the design

People have a chance to learn the very best way to work in the system, but it's ok if they learn that gradually over their first few patches. In fact if they're learning everything in their very first patch, the bar may be too high.

26 of 32

enjoyable first patches

Prompt, helpful reviews.

Don't move the goalposts.

Not too much refactoring.

Prefer integration tests.

People have a certain amount of goodwill and commitment. We want to do things that build it up, and not do things that waste it.

Delay inherently dissipates motivation and energy. If you come back and ask the contributor to do something promptly, the contributor is much more likely to do it than if you wait two weeks and then ask them. If you treat them as important, they are more likely to want to do something for you.

Even worse than delay: thinking you're done, and then being disappointed. One aspect is: moving the goalposts on an existing patch. I already suggested you should accept new contributors writing things in a more simplistic way than existing contributors. To take that a bit further: if you ask them to do some things, and they do them, then you should probably just accept their patch, even if you later think of other things that would be good. (Obviously, I don't mean you should accept it if you notice there's a nontrivial bug.)

Most new contributors are going to be relatively tentative in refactoring: even if they're comfortable in the language and environment (and they may not be), they may not have enough background state to know how the program typically does things, or they may not have enough confidence to propose a large patch and risk breaking something or ridicule.

A first patch is, for the contributor, an experiment: how is this project going to treat me? Make sure they get a good answer.

Users who put up a patch to fix a bug or add a feature will almost always have interactively tested their patch to make sure it does what they want.

But writing good tests, especially unit tests, can often be harder than writing the code: it requires finding out how to isolate the code under test, what level is most appropriate to test it at, perhaps also about how underlying levels fit together. Unit tests can have a lot of value.

So unit tests can be a big burden for new contributors. It is crucial that their patch not be rejected because of the lack of them. There are at least three things we can do:�

- work on the test framework to make it easy to add new tests

- accept patches that are tested, but not in the most elegant way

- or have an experienced developer write the unit tests and show them how it's done.

27 of 32

patch piloting

promise to help land patches
track the backlog
rota to make sure it's actually done
celebrate success

If you're convinced it is important to deal with contributed patches quickly and helpfully, how can you make sure this actually gets done amongst all the other noise?

We

1- Made a public commitment that if anyone puts up a reasonable attempt at a patch, we would help them get it finished off and landed. At minimum that means giving them prompt helpful feedback, but it can be much more: if they need tests and they don't have them, the pilot will if necessary write the tests to get it in.

2- Track the length of the outstanding merge proposals to make sure things aren't getting stalled. Ideally, you want to have tool support here that makes it clear what things are waiting on a core developer, which are waiting on someone else, which ought to be acted upon and which have been abandoned. If we're waiting on a non-core contributor and they seem to have not got back to the patch, we'll look at doing the work for them.

3- There are lots of other things competing for people's time, at first this can feel like a chore (but it gets better) and it's easy to assume someone else will do the reviews. So we set up a rota for employee developers - during that week, they were on point to make sure changes got landed.

4- Since it can feel like a bit of a chore, we want to recognize the work they do - people send a brief mail at the end showing what they got done. This also helps remind community contributors that we walk the extra mile to help them get their work in.

This is a nontrivial commitment of time, but it also makes a nontrivial improvement to the ease of contribution to the project, as measured by the Ubuntu developer survey.

28 of 32

but, but...!

- Are you asking me to merge things that I know are buggy?

No, of course not, if you see a bug in the patch you should say so.

Q: If we promise to finish things off for the contributor, couldn't they effectively DoS us by sending a stream of small patches, each of which uses up more time from the core developers than it takes them to generate?

A: Well, if you're sure someone is malicious, or trolling, you can start responding differently. And, perhaps if someone is not malicious but just enthusiastically inept, perhaps there will be a time when you have to start throttling your responses to them. But I haven't seen this yet.

On the other hand, if you're getting a continous stream of patches from new different developers - I think that's a really good thing. My theory is that at least some of them will "stick" and start doing code reviews themselves, and that should help you scale...

Q: But these patches are for really unimportant issues.

A: Well, one of your users thought it was important enough to spend their time working on it. That carries some information.

Q: But the patch is really not done the right way with regard to architecture, overall design, UI.

A: If it really would be too much work to rework it, it's ok to say so. Just try to reply quickly, without stalling, and to try to explain what would be the right way to add it.

29 of 32

go and try these:

design for scaling
effort tests
tested fakes
are we "done done"?
handoffs and delays before availability?
fast super-helpful reviews?
bugs: now, soon, or never?
order ≫ schedule

30 of 32

thanks!

lifeless, jml, flacoste, jelmer, jameinel, statik, vila, gz,

spiv, ianc, cinerama, abentley, stevea, sabdfl, ...

31 of 32

32 of 32

stable APIs or stable code?

Another thing that bit us badly was: trying to provide a stable in-process Python api, while improving the underlying code. Bazaar gave a rich API, which allowed some great hacks such as transparent access to foreign branches (git, svn, hg.)

Having the API was not bad. Promising it would remain stable was more questionable, because it imposed a burden on people working on Bazaar itself, without always achieving actual stability.

Python provides no real means to hide implementation details, and even if you use naming conventions, there's always an incentive on plugin developers to open the covers if that's what they need to do. It's also pretty hard to tell what semantics people might be depending upon. You see similar things inside the Python standard library, where some really pretty reasonable extensions to unittest or urlib2 require looking at the source and poking not-publicly-documented variables or methods.

I like, much better, what we eventually switched to: split off a stable release branch every six months, and get API stability by keeping the actual code stable. We put bug fixes back into that branch, but only fairly conservative ones that don't rearrange the code. On head, we have more freedom to change things - we don't need to make gratuitous changes, but it's one less constraint and one less mental overhead.