A few comments on recent work on Gadget.
Michael Stone
michael at laptop.org
Tue Jun 17 19:53:14 EDT 2008
Dear Daf & Guillaume,
I read through Guillaume's 'activity' branch a few days ago and recorded
several comments which I'd like to forward to you. I hope they help.
* Thanks for the nice documentation linked to from
http://wiki.laptop.org/go/Gadget
I really appreciate the effort spent here.
* It seems that we're using more naive algorithms than when I
last looked. activity_by_participants() is even marked FIXME. Have
you been able to execute any scaling tests on the server to see
whether these algorithms are problematic?
* We're still maintaining an entirely in-memory database.
- How much memory does each additional connection, view, activity,
etc. cost us?
- How will the component respond to low-memory conditions?
- Can we easily test this by rlimit()-ing the component separate
from the rest of the system?
* I see that much of the parsing code is wrapped in try/except blocks
which log parsing exceptions.
- Does the parsing code never throw other errors?
- If an uncaught exception propagates, how much of the component
dies? Is it just the connection which generated the exception or
will the whole component go down?
* Thanks for keeping the unit tests up to date with the rest of the
code.
- Any progress on random-data tests or on load/scaling tests?
* I see that we're beginning to use xpath queries instead of manually
destructuring our input.
- Are there any downsides to using xpath here?
* I'm glad to see the use of __slots__ to reduce memory usage of
several common objects.
- How much memory is consumed under each of your test loads?
- Have you done any testing for memory leaks?
- If not, how long is the longest period of time you've left the
component running under load (and idle?)?
* I see several places where we've got big if/elif trees.
- Perhaps these should turn into dictionary lookups or method
dispatch on objects?
* It seems that when handling an multi-part query, the server
accumulates all its results in memory, then serializes them, then,
transmits them.
- How much memory/time could be consumed by a big query?
- Could we profitably stream the results back to the client rather
than batching them?
* I infer that "views" contain stored queries and cached results. This
should be explained somewhere in the code. More generally, the code
should contain pointers to the relevent wiki documentation. (Or
vice-versa).
* The "search", "random", and "all" logic seems to be duplicated
throughout the code. We could profitably abstract this into a small
class hierarchy for queries.
* The activity_update() logic seems clunky to me. Why not just inform
all the views of the request and let them apply their own update
logic?
* I see a couple of instances of map and filter in this code.
Unfortunately, this isn't Haskell. Think about using generators or
regular iteration. Fuse your own loops.
* It seems that the Roster code doesn't adequately handle exceptions
which occur during file loading. Please comment.
* How confident in are you in the unicode-safety of your code. Why?
----
I'll try to get another person from 1cc or the general community
to review this code and to give you some more feedback. I've also
started reading the telepathy-gabble-gadget branch and will offer
feedback on it as I'm able. Please speak up loudly when you've got
something that you like someone else to review.
Thanks,
Michael
More information about the Devel
mailing list