Tree can go red on "sizes" with no way to fix it |
||||||
Issue descriptionSizes just went red on the Mac, bug 831951 . It seems like that step was just unbroken. The expectation is 0 and now the actual number of static initializers is nonzero. There is literally no way to reset that number. The number lives in perf_expectations.json. There is a link in that file to http://www.chromium.org/developers/tree-sheriffs/perf-sheriffs . That linked page has zero information on this number. I emailed the perf team. They said: "Wow, the vast majority of these were replaced with the perf dashboard in 2011. The old code for generating expectations is here: https://cs.chromium.org/chromium/tools/perf/make_expectations.py "But I have no idea how it works anymore." I tried running make_expectations.py. It doesn't work and has no documentation. This is BAD. I'm going to hack together a change the expectations file, but I shouldn't have to. This is literally unmaintained (the perf team disavows it) yet can break the tree.
,
Apr 13 2018
I have to add NOPRESUBMIT=true. There are presubmits checking the format of this file but no working tools to generate this format. Argh!
,
Apr 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/00d3a24514b59d9628b2074ed2f2d82d4d1953de commit 00d3a24514b59d9628b2074ed2f2d82d4d1953de Author: Avi Drissman <avi@chromium.org> Date: Fri Apr 13 19:45:37 2018 Set the expected number of initializers to 2 on the Mac. This was broken for a very long time and the initializers regressed. There was a tool to update the perf_expectations.json file, but it is either broken or has zero documentation. The perf team disavows it. This is unmaintained, so I'm manually hacking this file to get it to a state in which it works. BUG= 831951 , 832854 NOTRY=true NOPRESUBMIT=true TBR=thakis@chromium.org Change-Id: Idf1bd61d40540d0b85497e6669297fb54c2c2a28 Reviewed-on: https://chromium-review.googlesource.com/1012743 Commit-Queue: Avi Drissman <avi@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#550723} [modify] https://crrev.com/00d3a24514b59d9628b2074ed2f2d82d4d1953de/tools/perf_expectations/perf_expectations.json
,
Apr 13 2018
I don't have cycles to look at this. It used to work; someone broke it (and then left the team a while later). Back to mac triage.
,
Apr 13 2018
Also the existing initializers need to be removed again, not sure which bug tracks that.
,
Apr 13 2018
The otool work and the return to 0 work is bug 831951 . This bug is about this CRITICAL STEP in the build being UNOWNED. NO ONE is able to fix this or adjust the numbers that it wants. Being unowned is bad. This needs ownership or it should be removed.
,
Apr 13 2018
I don't know that I'd consider this a critical step ... I agree that we should turn it off if there isn't an active maintainer.
,
Apr 13 2018
,
Apr 13 2018
It looks like we were attempting to simplify this step in bug 572393, so I'm going to dup this into that and get that bug fixed instead.
,
Apr 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bc0d927cd391a0345979b6b12c58019f9a29bcb2 commit bc0d927cd391a0345979b6b12c58019f9a29bcb2 Author: Dirk Pranke <dpranke@chromium.org> Date: Fri Apr 13 23:00:22 2018 Bump mac static initializer count to 4 to work around bug. It looks like when we're computing the number of static initializers on mac we're still assuming a 32-bit word and not the 64-bit words that we actually use, and so we mis-report the number of initializers as twice what they actually are. All of this perf expectation code needs to be ripped out and replaced with some dumb static checks, but for now, this CL just updates the expected number from 2 (the correct number) to 4, and updates the checksums in the json file to get the presubmit to pass. BUG= 831951 , 832854 TBR=avi@chromium.org, wolenetz@chromium.org, thakis@chromium.org NOTRY=true NOTREECHECKS=true Change-Id: Ie6de49af3cc1443b8acc36ce80946ea50e5a76ab Reviewed-on: https://chromium-review.googlesource.com/1013205 Commit-Queue: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#550768} [modify] https://crrev.com/bc0d927cd391a0345979b6b12c58019f9a29bcb2/tools/perf_expectations/make_expectations.py [modify] https://crrev.com/bc0d927cd391a0345979b6b12c58019f9a29bcb2/tools/perf_expectations/perf_expectations.json
,
Apr 13 2018
"I don't know that I'd consider this a critical step ... I agree that we should turn it off if there isn't an active maintainer." I was imprecise. I also don't personally consider this a "critical" step, but the fact that it can make a bot red means that our system considers it critical. In any case, thank you very much for your assistance in getting this green.
,
Apr 14 2018
> the fact that it can make a bot red means that our system considers it critical. We don't really have a way to distinguish the different levels of failure in the system, but I wouldn't call them all "critical" as a result. However, that's just nit-picking :).
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/00d3a24514b59d9628b2074ed2f2d82d4d1953de commit 00d3a24514b59d9628b2074ed2f2d82d4d1953de Author: Avi Drissman <avi@chromium.org> Date: Fri Apr 13 19:45:37 2018 Set the expected number of initializers to 2 on the Mac. This was broken for a very long time and the initializers regressed. There was a tool to update the perf_expectations.json file, but it is either broken or has zero documentation. The perf team disavows it. This is unmaintained, so I'm manually hacking this file to get it to a state in which it works. BUG= 831951 , 832854 NOTRY=true NOPRESUBMIT=true TBR=thakis@chromium.org Change-Id: Idf1bd61d40540d0b85497e6669297fb54c2c2a28 Reviewed-on: https://chromium-review.googlesource.com/1012743 Commit-Queue: Avi Drissman <avi@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#550723} [modify] https://crrev.com/00d3a24514b59d9628b2074ed2f2d82d4d1953de/tools/perf_expectations/perf_expectations.json
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bc0d927cd391a0345979b6b12c58019f9a29bcb2 commit bc0d927cd391a0345979b6b12c58019f9a29bcb2 Author: Dirk Pranke <dpranke@chromium.org> Date: Fri Apr 13 23:00:22 2018 Bump mac static initializer count to 4 to work around bug. It looks like when we're computing the number of static initializers on mac we're still assuming a 32-bit word and not the 64-bit words that we actually use, and so we mis-report the number of initializers as twice what they actually are. All of this perf expectation code needs to be ripped out and replaced with some dumb static checks, but for now, this CL just updates the expected number from 2 (the correct number) to 4, and updates the checksums in the json file to get the presubmit to pass. BUG= 831951 , 832854 TBR=avi@chromium.org, wolenetz@chromium.org, thakis@chromium.org NOTRY=true NOTREECHECKS=true Change-Id: Ie6de49af3cc1443b8acc36ce80946ea50e5a76ab Reviewed-on: https://chromium-review.googlesource.com/1013205 Commit-Queue: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#550768} [modify] https://crrev.com/bc0d927cd391a0345979b6b12c58019f9a29bcb2/tools/perf_expectations/make_expectations.py [modify] https://crrev.com/bc0d927cd391a0345979b6b12c58019f9a29bcb2/tools/perf_expectations/perf_expectations.json |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by a...@chromium.org
, Apr 13 2018tests/perf_expectations_unittest.py (0.80s) failed .F. ====================================================================== FAIL: testNoUpdatesNeeded (__main__.PerfExpectationsUnittest) ---------------------------------------------------------------------- Traceback (most recent call last): File "tests/perf_expectations_unittest.py", line 154, in testNoUpdatesNeeded msg='Update expectations first by running ./make_expectations.py') AssertionError: Update expectations first by running ./make_expectations.py ---------------------------------------------------------------------- Ran 3 tests in 0.181s FAILED (failures=1) Yet... /V/s/c/src (sizes) [127]> tools/perf_expectations/make_expectations.py linux-release-64/sizes/chrome-si/initializers: ERROR: cannot find json data, please verify Now I have to bypass presubmits. This is nuts.