[08:56] <stub> lifeless: 'An iterable of slices' seems rather overkill, and possibly self defeating. What is the advantage of this over just (min_index, max_index) ?
[09:01] <lifeless> stub: do you mean just special case the exact situation?
[09:02] <stub> Do we ever see more than one slice passed in?
[09:02] <lifeless> yes
[09:02] <stub> If so, are the holes almost always a single comment?
[09:02] <lifeless> the slices are
[09:02] <lifeless> [:40], [-40:]
[09:02] <lifeless> more or less
[09:03] <stub> ok. That makes sense then.
[09:04] <stub> If the holes were small, we could be better off pulling them and skipping unwanted ones client side than the more complex WHERE clause.
[09:04] <lifeless> yeah
[09:04] <lifeless> the prototypical case is bug 1
[09:04] <_mup_> Bug #1: Microsoft has a majority market share <iso-testing> <ubuntu> <Clubdistro:Confirmed> <Computer Science Ubuntu:Invalid by compscibuntu-bugs> <EasyPeasy Overview:Invalid by ramvi> <GNOME Screensaver:Won't Fix> <Ichthux:Invalid by raphink> <JAK LINUX:Invalid> <The Linux OS Project:In Progress> <metacity:In Progress> <OpenOffice:In Progress by lh-maviya> <Tabuntu:Invalid by tinarussell> <Tivion:Invalid by shakaran> <Tv-Player:New> <Ubun
[09:04] <lifeless> where the hole is 1200 rows long
[09:05] <lifeless> stub: https://bugs.launchpad.net/launchpad/+bug/607935/comments/12
[09:05] <_mup_> Bug #607935: timeout on bugtask:+index <lp-bugs> <pg83> <timeout> <Launchpad itself:Triaged> < https://launchpad.net/bugs/607935 >
[09:06] <lifeless> stub: doing two queries is 40ms total; one with the complex clause 30ms total
[09:07] <stub> set(row[1].ownerID for row in rows if row[1].ownerID)
[09:08] <lifeless> rather than the discard? its slower
[09:09] <lifeless> doing a discard is one check for None, discarding in the loop is a check per row
[09:10] <stub> Any reason you want to keep eager_load_watches() in there?
[09:10] <lifeless> oh
[09:11] <lifeless> thinko, will delete.
[09:16] <stub> lifeless: Hmm... if you do that, doesn't that mean build_comments_from_chunks() will be loading bugwatches one at a time from the DB?
[09:16] <lifeless> no
[09:16] <lifeless> well
[09:16] <lifeless> we use that in one place only
[09:16] <lifeless> and that place is a bug context
[09:16] <lifeless> bugs have already preloaded all bug watches.
[09:17] <lifeless> eager_load_watches is already commented out to be uncalled
[09:21] <stub> Right. I've asked for a comment anyway - when I see code like that I assume database impedance mismatch issues and one-query-per-iteration performance because I don't know the call path.
[09:21] <stub> And tests for slicing behavior.
[09:27] <lifeless> fair enough
[09:27] <lifeless> that method is /barely/ tested already; I'd like to nuke it entire for indexed_messages
[09:27] <lifeless> but ETIME
[09:28] <lifeless> I'll add some tests tomorrow morning
=== henninge_ is now known as henninge
=== wallyworld___ is now known as wallyworld