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