[14:59] <statik> hello, welcome to the reviewer meeting for people who are attending at this time
[15:00] <mwhudson> hello statik
[15:00] <statik> barry asked me to help with this meeting as he is away
[15:00] <statik>     * Roll call
[15:00] <statik>     * Next meeting
[15:00] <statik>     * Action items
[15:00] <statik>     * Queue status
[15:00] <statik>     * Mentoring update
[15:00] <statik>     * General Queue allocations (w/salgado and lifeless on vaca)
[15:00] <statik>     * Review process changes
[15:00] <statik>           o Tool update
[15:00] <statik>           o Recruit reviews
[15:00] <statik> so, who is here (please ping anyone you know should be here)
[15:00] <intellectronica> me
[15:00] <gmb> me
[15:00] <mwhudson> me
[15:00] <statik> me
[15:00] <sinzui> me
[15:00] <BjornT> me
[15:01] <flacoste> me
[15:01] <ddaa> me
[15:01] <flacoste> salgado is on leave
[15:01] <statik> bac is on leave
[15:01] <flacoste> barry is at the dentist
[15:02] <statik> next meeting, same time?
[15:02]  * ddaa complains about reviewing 1400 lines soyuz diff of doom which appears to consist mostly of pagetests improvement although it seems the branch was intended to fix a bug.
[15:02] <mwhudson> jtv: here?
[15:02] <statik> ok, next meeting is at the same time
[15:02] <statik> action items
[15:02]  * sinzui recalls the 600 line soyuz diff that inflated to 1600 lines.
[15:02] <jtv> mwhudson: here
[15:03] <statik> barry was supposed to edit the wiki about on-call procedures, announce the new 800 line limit on branches, and email launchpad-reviews about adding a joint meeting with asiapac, perhaps once a cycle
[15:03] <statik> barry told me he would do those things later today
[15:03] <statik> intellectronica had an action item to work on a cover letter template, how is that going?
[15:04] <intellectronica> sorry, i'm rubbish. maybe later today? if not then next year
[15:04] <statik> fantastic, thanks for playing
[15:04] <sinzui> There was an announcement about the 800 line restriction
[15:04] <statik> ddaa, sinzui, hopefully the announcement about 800 line limit later today will help prevent your situations
[15:04] <statik> oh, then I missed it
[15:04] <statik> queue status!
[15:04] <statik> has anyone seen the queue?
[15:05] <statik> I see 3 in the general queue
[15:05] <statik> intellectronica: how is your load today?
[15:05] <intellectronica> i'm going through the queue. nearly done with cprov's branch and i hope to take at least another one off the queue
[15:06] <ddaa> oh, my branch of doom is from cprov too
[15:06] <sinzui> ddaa: So was mine.
[15:06] <ddaa> I can see a pattern.
[15:06] <statik> intellectronica: super. barry suggested that with lifeless and salgado on vacation we need to be careful to do review allocations
[15:06] <statik> ddaa: if the branch is too big then reject it
[15:07] <mwhudson> the only old branch in the queue seems to be adeuring/launchpad/parse-hwdb-submissions-step1, assigned to jtv
[15:07] <jtv> mwhudson: arrived too late for me last night, doing it now
[15:07] <intellectronica> i can allocate when i'm done for the day
[15:07] <mwhudson> so i think the queue status is "good"
[15:07] <ddaa> statik: I'll give it its three hours to see how far I can go.
[15:07] <statik> intellectronica: great. it would be good if all on-call reviewers can do the same
[15:08] <statik> ddaa: that sounds like a good balance to strike. I'll remind cprov about the 800 line limit
[15:08] <statik> next item, mentoring update!
[15:08] <statik> are the folks who are being mentored getting any branches to review?
[15:08] <gmb> Yes.
[15:08] <ddaa> got two
[15:09] <gmb> And I'm going to share on-call duties with mwhudson tomorrow.
[15:09] <jtv> mine's a big one
[15:09] <mwhudson> i think barry assigned a bunch of reviews to mentorees last night
[15:09] <mwhudson> i was just starting to mentor gmb's reviews when the meeting started
[15:10] <statik> excellent, sounds like that is going ok then
[15:10] <statik> final item, review process changes
[15:10] <statik> or, tool update
[15:10] <statik> does anyone have anything to say about tools?
[15:10] <mwhudson> i fixed a couple of the issues in my lpreview plugin this morning
[15:10] <mwhudson> so update it, please
[15:10] <statik> I saw mwhudson fixed a lot of stuff ,but I still haven't tried using it yet
[15:11] <mwhudson> i should add an item to the dev meeting tomorrow about asking if people have used it
[15:11] <statik> mwhudson: is it going to incorporate intellectronica's cover letter template?
[15:11] <mwhudson> statik: when he writes it, yes :)
[15:12] <flacoste> mwhudson: add an item to the agenda
[15:12] <ddaa> thanks for merging my fix
[15:12] <flacoste> that's a goo didea
[15:12] <statik> ok, I guess thats it for tools
[15:12] <statik> I suggest 3 minutes of random comments from anyone before we adjourn the meeting
[15:12]  * ddaa raises hand
[15:12] <statik> have I mentioned how much I like the on-call reviewer process?
[15:13] <ddaa> after flacoste bothered me yesterday
[15:13] <statik> go for it ddaa
[15:13] <ddaa> I wrote a branch to merge all ftests directory into tests directories
[15:13] <mwhudson> flacoste: done
[15:13] <ddaa> it's going to conflict with nearly about everything
[15:13] <ddaa> It already bounced once from pqm because a patch got in before it that added a new ftests file.
[15:13] <jtv> ddaa: even the test infrastructure stuff?  I like having that slightly separate.
[15:13] <statik> ddaa: we could probably coordinate with kiko and mthaddon to have your branch land first thing after the rollout
[15:14] <ddaa> jtv: flacoste said it should move to "testing" directories
[15:14] <jtv> ah
[15:14] <ddaa> statik: in the meantime, I'll just keep trying to put it in pqm
[15:14] <ddaa> but I'm afraid it might cause some branches to miss the end-of-week deadline.
[15:15] <flacoste> might be better to land it then
[15:15] <ddaa> statik: but that sounds like a good idea.
[15:15] <flacoste> then being after the roll-out
[15:15] <ddaa> ok
[15:15] <statik> yeah, I see 7 scripts in PQM now, if we know that branch is conflicty then doing it during that quiet period would be better
[15:15] <mwhudson> it's not really a reviewer meeting item, but pqm runs are getting slower at a depressing rate
[15:16] <statik> mwhudson: it's all that email that flacoste is sending to the list
[15:16] <statik> about PQM timing
[15:16] <ddaa> did not see that
[15:16] <ddaa> but I want to say
[15:16] <ddaa> I think the current pqm timing data
[15:16] <ddaa> is almost useless
[15:16] <ddaa> it's measuring the wrong thing
[15:17] <ddaa> you can trivially tweak it by splitting or combining pagetests
[15:17] <statik> I think it would be worth organizing a separate discussion to talk about PQM timing. would someone here volunteer to organize such a meeting?
[15:17] <ddaa> what we care about is how much time is spend in the code that's actually called from the pagetests
[15:17] <statik> ddaa: that is a good point
[15:18] <BjornT> ddaa: we also care about how fast the test suite runs as a whole, don't we?
[15:18] <flacoste> i don't understand
[15:18] <flacoste> ddaa: you are talking about the aggregated time of PageTests or the one of individual test?
[15:18] <statik> the current emails focus a bit on the slowest tests, I think ddaa was pointing out that the size of an individual test is less important than the overall time
[15:18] <ddaa> BjornT: the current pqm run summary does to help see how that evolves, and I do not think it was intended for that. Tom's pqm timing graph is good for that.
[15:19] <ddaa> statik: right
[15:19] <ddaa> what matters more is "what makes all the tests slow"
[15:19] <mwhudson> yes, it was https://devpad.canonical.com/~mthaddon/pqm_durations.html that inspired my comment
[15:19] <flacoste> the email gives an hint to that
[15:19] <BjornT> ddaa: oh, i thought you were talking about tom's graph
[15:19] <ddaa> i.e. is there some specific helper function that accounts for 25% of the runtime? We do not know?
[15:19] <flacoste> 20% is spent in resetting database
[15:20] <statik> flacoste: since you are sending the current PQM timing mails, I assume you have some plans or ownership of this issue, so if you think we should have a discussion about PQM times and how to make further progress, would you organize it?
[15:20] <statik> (outside of the reviewer meeting I mean)
[15:20] <flacoste> statik: i could, but I don't think a meeting is necessary
[15:21] <statik> flacoste: I trust your judgement completely
[15:21] <flacoste> somebody need to profile
[15:21] <flacoste> and then we can talk about solutions
[15:21] <statik> that's all for this weeks reviewer meeting, I'll do a countdown now because I like the dramatic effect
[15:21] <statik> 5
[15:21] <flacoste> without profiling data, everything is guessing
[15:21] <statik> 4
[15:21] <statik> 3
[15:21] <statik> 2
[15:21] <jtv> "it's going to blow!"
[15:21] <statik> 1.4
[15:21] <statik> 0
[15:21] <statik> thanks everyone!
[15:21] <jtv> thanks
[15:21] <flacoste> thanks statik for a short meeting!
[15:22] <ddaa> mwhudson: that's indeed a depressing sight
[15:22] <mwhudson> thanks statik
[15:22] <flacoste> ddaa: you may want to try merging the 3 branches before yours
[15:22] <flacoste> ddaa: in PQM
[15:22] <ddaa> flacoste: I tried :)
[15:23] <flacoste> ddaa: did it work?
[15:23] <ddaa> no tree shape conflict
[15:23] <flacoste> ok, so it could work
[15:23] <ddaa> might be content conflicts through (new references to ftests)
[15:23] <ddaa> did not check that
[15:23] <flacoste> right
[15:23] <ddaa> I did not even check that the full test suite passes in my branch.
[15:23] <flacoste> if it fails, better than to postpone
[15:23] <ddaa> ack
[15:25] <ddaa> flacoste: I believe the db reset cost cannot easily be fixed
[15:25] <flacoste> probably not
[15:25] <ddaa> since the test adapter already just do abort if there was no commit
[15:26] <flacoste> i tried running the test suite under the profiler
[15:26] <flacoste> it trashed my machine to death
[15:26] <ddaa> that's usually a good indication that profiling is needed :)
[15:26] <flacoste> well
[15:27] <flacoste> the problem is with the profiler
[15:27] <flacoste> i think it holds all its data structure in memory
[15:27] <flacoste> so you might guess that with the size of LP test-suite...
[15:27] <ddaa> which profiler did you use?
[15:27] <flacoste> ./test.py --profile
[15:27] <flacoste> which uses hotspot i think
[15:27] <ddaa> hotshot
[15:27] <flacoste> right
[15:28] <ddaa> IIRC, at some point hotshot worked by writing a ton of data on disk
[15:28] <ddaa> then postprocessing it
[15:28] <ddaa> if I'm correct, it might be possible to get that behaviour back
[15:29] <ddaa> it might have been disabled to fit into the old legacy profile api.
[15:30] <ddaa> it's a strategy for low-impact profiling: externalize the collating of the profiling data so it does not impact the cpu times during profiling.
[15:30] <mwhudson> there is also lsprof/cProfile
[15:30] <mwhudson> which is less demented than hotshot in this regard
[15:31] <mwhudson> ddaa: the problem with hotshot is that _loading the profile data_ takes as long as running the original program under the python profilee
[15:31] <ddaa> here the problem is not one of time
[15:31] <ddaa> but one of space
[15:31] <ddaa> flacoste was just not able to complete the run because of excessive memory use.
[15:32] <ddaa> so it makes sense to aggressively trade space for time.
[15:33] <ddaa> my comment about low-impact profiling was to underline that we can expect to find this feature is a number of profilers.