analyze.py heuristics might cause the perf waterfall to see effects delayed from the culprit CL |
|||
Issue descriptionFollow-up from conversation in https://codereview.chromium.org/2707363006/ Not necessarily a high prio thing, but let's keep track of this somewhere in the bugtracker. ----- primiano@ Wait, isn't analyze.py using the dependency graph generated by the compiler? depending on GN files seems really fragile, especially for headers which are added only in one .gn file but they are referenced by various translation units in several targets. dpranke@ No, we don't directly use the graph generated by the compiler, because in order to get that graph we'd have to actually do the compile, which would defeat much of the point. `gn check` parses c++ and header files to do a rough approximation of the results, but it is only rough (again, to be fast). There are discussions about how to sync things up w/ the dependency graphs from the compilers but we haven't yet figured out a great way to do that that is both up-to-date, too expensive, and maintainable. ----- Re: you have to run the compiler. I thought you had to run only the preprocessor, but now I'm adventuring myself in an area where I have very little clues. +thakis might have insights here. If I am reading this correctly, this means that when somebody lands a change to a header that is not correctly placed in gn files, the real effect of the CL might be measured only later in the stage and attributed to the wrong CL (unless chromium.perf builders build from clean all the time?) Out of curiosity, how does it work with java? does it follow transitive dependencies? I don't even know if we still use globs in build files for java these days. We had bunch of occasions of java heap memory regressions that were blamed to clearly unrelated CLs. I wonder if there is any subtlety here that might cause this to happen more frequently with java changes? (+agrieve)
,
Feb 23 2017
> The perf waterfalls don't even run analyze. I mean the builders in chromium.perf, don't they? (I don't seem able to open https://build.chromium.org/p/chromium.perf from off corp to check) > If your intent is to file a bug specifically about how analyze works, Well I guess that the effect is more subtle for perf. In the main waterfall at worse the effect will show at some point and should be easier to figure out the culprit from the breakage, even if unrelated. In the case of perf, the cause-effect relationship might be way more subtle and undetectable. I opened this mostly to keep track and refer in perf bugs if I'll find any suspect case.
,
Feb 23 2017
but yeah -thakis, plz ignore this. Looks like all the discussion about analyze happened already in Issue 661774
,
Feb 23 2017
No, the builders on chromium.perf do not run analyze. Only the tryserver run analyze. The purpose of analyze is to try and figure out which targets are affected by the files in a given patch, so that we can distinguish targets that are out-of-date because of changes in a patch vs. targets that are out of date because unrelated changes have landed. In other words, we try to avoid running browser_tests on all platforms for a patch that is only affecting a java file on android. So, running analyze on continuous builders would be counter-productive. The actual building of the targets to ensure they are correct and up-to-date is still done by ninja using the full dependency graph as you'd expect.
,
Feb 23 2017
Aaaaa so this is all a big brainfart. apologies for the noise. (a part of me was desperately looking for a plausible explanation for those weird regressions. I will have to look somewhere else). Closing this as the general analyze.py is already tracked in Issue 661774. Thanks for the thorough (as usual) explanation Dirk!
,
Feb 23 2017
no problem, you're welcome :). |
|||
►
Sign in to add a comment |
|||
Comment 1 by dpranke@chromium.org
, Feb 23 2017