=== cprov-away is now known as cprov | ||
=== cprov is now known as cprov-lunch | ||
statik | hello, welcome to the reviewer meeting for people who are attending at this time | 14:59 |
---|---|---|
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:00 |
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:01 |
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:02 |
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:03 |
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:04 |
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:05 |
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:06 |
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:07 |
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:08 |
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:09 |
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:10 |
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:11 |
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:12 |
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:13 |
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:14 |
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:15 |
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:16 |
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:17 |
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:18 |
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:19 |
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:20 |
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:21 |
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:22 |
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:23 |
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:25 |
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 |
=== cprov-lunch is now known as cprov | ||
flacoste | well | 15:26 |
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:27 |
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:28 |
ddaa | it might have been disabled to fit into the old legacy profile api. | 15:29 |
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:30 |
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:31 |
ddaa | so it makes sense to aggressively trade space for time. | 15:32 |
ddaa | my comment about low-impact profiling was to underline that we can expect to find this feature is a number of profilers. | 15:33 |
=== cprov is now known as cprov-away |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!