launchpad-dev team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

One common performance killing pattern is dereferencing non-cached
References on a sequence of model objects. A stupid example might be:

    owners = [bug.owner for bug in lots_of_bugs]

Instead we're learning to write this like:

    owners = bulk.load_related(Person, lots_of_bugs, "ownerID")

or other ways that work out roughly the same (for example,
getPrecachedPersonsFromIDs() is probably a better fit here).

However, if we don't spot these problems during development or review,
we end up noticing them in OOPS reports. This is suboptimal.

One approach to improving this situation could be to forbid (with
code) the use of Reference properties that are not already cached (in
the store cache; this is nothing to do with @cachedproperty).

Doing this everywhere is not going to fly, at least right now -
there's probably way too much code that would break if we did this -
and sometimes we would want to permit their use - when we're just
looking up a single object for example.

We could make it possible to conditionally restrict their use, in
critical sections for example. We could use decorators and context
managers for convenience to help us ratchet down their use.

It so happens that I knocked up a quick Storm branch earlier to better
explain the idea:

  lp:~allenap/storm/blocked-references

  Diff: http://paste.ubuntu.com/603313/

It's fairly rough and ready, but it should work, and there are even
some tests. Here's a fairly contrived example:

    from storm.references import Reference

    with Reference.blocked():
        owners = get_bug_owners(lots_of_bugs)

If anything in get_bug_owners() deferences a non-cached Reference
property it'll blow up with ConnectionBlockedError.

There's also a Reference.unblocked() context manager for when you're
deep in the called code and you must use a Reference. For example:

    with Reference.unblocked():
        actor = self.owner.preferredemail.email

Providing a decorator for these two would be fairly trivial.

Before taking this any further I am interested in your views. Can you
think of places where you could, or could have, used this? Is the
granularity right, or should we be able to restrict just individual
properties, or all properties on a model class? Is the idea okay but
the implementation going about it all wrong?

Gavin.