check_network_annotations is too slow on the trybots |
||||
Issue descriptionIt looks like the "check_network_annotations" step takes ~3:30 on "Linux Builder", which isn't great but is probably acceptable. However, it also takes almost 20 minutes on linux_chromium_rel_ng, and that's way too slow (presumably windows would be even worse). We need to figure out how to make this check run in a couple of minutes, tops, before we can run it in the CQ. I'm disabling it in the meantime. I apologize for not noticing this earlier :(.
,
Nov 23 2017
,
Nov 24 2017
The test runs in 40s on a z840 machine with 48 cores, 64GB RAM, and SSD harddrive. Given that the trybot virtual machines are not as capable, maybe I need to move it to swarming bots.
,
Nov 24 2017
Unfortunately, given that you need to scan source code, I don't think we can easily move it to swarming. At the very least, we'd have to be able to identify a subset of the tree to scan.
,
Dec 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dffd5dc48c06a528e891868f3af7ef91d9950b94 commit dffd5dc48c06a528e891868f3af7ef91d9950b94 Author: rhalavati@google.com <rhalavati@google.com> Date: Mon Dec 11 15:39:23 2017 Extend annotations.xml. Extends annotations.xml to include incomplete annotations, file paths, and the list of fields in the incomplete annotations. Bug: 656607 Bug: 788035 Change-Id: Ibabbbd91fa8d618571d3fe00dc32fd0aec0277ed Reviewed-on: https://chromium-review.googlesource.com/817749 Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Reviewed-by: Martin Šrámek <msramek@chromium.org> Cr-Commit-Position: refs/heads/master@{#523102} [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/metrics/histograms/enums.xml [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/auditor/instance.cc [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/auditor/instance.h [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/auditor/traffic_annotation_auditor.h [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/auditor/traffic_annotation_exporter.cc [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/auditor/traffic_annotation_exporter.h [modify] https://crrev.com/dffd5dc48c06a528e891868f3af7ef91d9950b94/tools/traffic_annotation/summary/annotations.xml
,
Dec 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/437ba3c4c4987abb77b5907e3bce97642ba917ab commit 437ba3c4c4987abb77b5907e3bce97642ba917ab Author: rhalavati@google.com <rhalavati@google.com> Date: Wed Dec 13 13:27:25 2017 Run Traffic Annotation tests on Trybot only on modified files. Traffic annotations test script on trybot is updated to only consider the list of changed files. Bug: 788035 Bug: 656607 Change-Id: Ib02f971edc65076c44f4aecf0589b30d381e00f7 Reviewed-on: https://chromium-review.googlesource.com/813635 Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Reviewed-by: Martin Šrámek <msramek@chromium.org> Cr-Commit-Position: refs/heads/master@{#523761} [modify] https://crrev.com/437ba3c4c4987abb77b5907e3bce97642ba917ab/tools/traffic_annotation/scripts/check_annotations.py
,
Jan 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/36e99a557191d54179c546cc9fdcd323508a2365 commit 36e99a557191d54179c546cc9fdcd323508a2365 Author: rhalavati@google.com <rhalavati@google.com> Date: Mon Jan 08 09:15:08 2018 Activate Network traffic annotation tests for Linux trybot. Traffic annotation tests are updated to run on the changed files instead of the whole repository, which should make tests much faster. Bug: 690323 Bug: 788035 Change-Id: Ie67c1abe02d2f1670e1d5c0b4491682157f390c1 Reviewed-on: https://chromium-review.googlesource.com/842403 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Martin Šrámek <msramek@chromium.org> Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Cr-Commit-Position: refs/heads/master@{#527588} [modify] https://crrev.com/36e99a557191d54179c546cc9fdcd323508a2365/tools/traffic_annotation/README.md [modify] https://crrev.com/36e99a557191d54179c546cc9fdcd323508a2365/tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1 [modify] https://crrev.com/36e99a557191d54179c546cc9fdcd323508a2365/tools/traffic_annotation/scripts/check_annotations.py
,
Jan 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96de83c487be35a0ca3d55be03198a6bebc4a7f7 commit 96de83c487be35a0ca3d55be03198a6bebc4a7f7 Author: John Abd-El-Malek <jam@chromium.org> Date: Mon Jan 08 15:30:21 2018 Revert "Activate Network traffic annotation tests for Linux trybot." This reverts commit 36e99a557191d54179c546cc9fdcd323508a2365. Lots of failures on CQ like this: Running ['/usr/bin/python', '/b/c/b/linux/src/tools/traffic_annotation/scripts/check_annotations.py', '--build-path', u'/b/c/b/linux/src/out/Release'] in None (env: None) [Errors] (1) 'tools/traffic_annotation/summary/annotations.xml' requires update. It is recommended to run traffic_annotation_auditor locally to do the updates automatically (please refer to tools/traffic_annotation/auditor/README.md), but you can also apply the following edit(s) to do it manually: Update line: ' <item id="download_web_contents_frame" hash_code="56351037" type="0" content_hash_code="3657889" os_list="linux,windows" file_path="content/browser/web_contents/web_contents_impl.cc"/>' --> ' <item id="download_web_contents_frame" hash_code="56351037" type="0" content_hash_code="3657889" os_list="windows" file_path="content/browser/web_contents/web_contents_impl.cc"/>' If you are using build flags that modify files (like jumbo), rerun the auditor using --all-files switch. Command ['/usr/bin/python', '/b/c/b/linux/src/tools/traffic_annotation/scripts/check_annotations.py', '--build-path', u'/b/c/b/linux/src/out/Release'] returned exit code 1 step returned non-zero exit code: 1 Original change's description: > Activate Network traffic annotation tests for Linux trybot. > > Traffic annotation tests are updated to run on the changed files instead > of the whole repository, which should make tests much faster. > > Bug: 690323 > Bug: 788035 > Change-Id: Ie67c1abe02d2f1670e1d5c0b4491682157f390c1 > Reviewed-on: https://chromium-review.googlesource.com/842403 > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > Reviewed-by: Martin Šrámek <msramek@chromium.org> > Commit-Queue: Ramin Halavati <rhalavati@chromium.org> > Cr-Commit-Position: refs/heads/master@{#527588} TBR=dpranke@chromium.org,msramek@chromium.org,rhalavati@chromium.org Change-Id: If2a8b71463dcfd4d298a125925d64877a78e6c86 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 690323 , 788035 Reviewed-on: https://chromium-review.googlesource.com/854073 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#527637} [modify] https://crrev.com/96de83c487be35a0ca3d55be03198a6bebc4a7f7/tools/traffic_annotation/README.md [modify] https://crrev.com/96de83c487be35a0ca3d55be03198a6bebc4a7f7/tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1 [modify] https://crrev.com/96de83c487be35a0ca3d55be03198a6bebc4a7f7/tools/traffic_annotation/scripts/check_annotations.py
,
Jan 8 2018
I've reverted this due to the failures seen. Given that this has caused 2 problems on the CQ each time it was enabled, please monitor the trybots closely next time when renabling it.
,
Jan 8 2018
btw here are some examples: https://chromium-review.googlesource.com/c/chromium/src/+/846611 gave https://ci.chromium.org/buildbot/tryserver.chromium.linux/linux_chromium_rel_ng/618971 Running ['/usr/bin/python', '/b/c/b/linux/src/tools/traffic_annotation/scripts/check_annotations.py', '--build-path', u'/b/c/b/linux/src/out/Release'] in None (env: None) [Errors] (1) 'tools/traffic_annotation/summary/annotations.xml' requires update. It is recommended to run traffic_annotation_auditor locally to do the updates automatically (please refer to tools/traffic_annotation/auditor/README.md), but you can also apply the following edit(s) to do it manually: Update line: ' <item id="download_web_contents_frame" hash_code="56351037" type="0" content_hash_code="3657889" os_list="linux,windows" file_path="content/browser/web_contents/web_contents_impl.cc"/>' --> ' <item id="download_web_contents_frame" hash_code="56351037" type="0" content_hash_code="3657889" os_list="windows" file_path="content/browser/web_contents/web_contents_impl.cc"/>' If you are using build flags that modify files (like jumbo), rerun the auditor using --all-files switch. Command ['/usr/bin/python', '/b/c/b/linux/src/tools/traffic_annotation/scripts/check_annotations.py', '--build-path', u'/b/c/b/linux/src/out/Release'] returned exit code 1 but this change didn't change that code that makes the request. Another example: https://chromium-review.googlesource.com/c/chromium/src/+/842863 https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_rel_ng%2F618947%2F%2B%2Frecipes%2Fsteps%2Fcheck_network_annotations__with_patch_%2F0%2Fstdout (1) 'tools/traffic_annotation/summary/annotations.xml' requires update. It is recommended to run traffic_annotation_auditor locally to do the updates automatically (please refer to tools/traffic_annotation/auditor/README.md), but you can also apply the following edit(s) to do it manually: Update line: ' <item id="navigation_url_loader" hash_code="63171670" type="0" content_hash_code="129352907" os_list="linux,windows" file_path="content/browser/loader/navigation_url_loader_network_service.cc"/>' --> ' <item id="navigation_url_loader" hash_code="63171670" type="0" content_hash_code="129352907" os_list="windows" file_path="content/browser/loader/navigation_url_loader_network_service.cc"/>' Update line: ' <item id="resource_dispatcher_host" hash_code="81157007" type="0" content_hash_code="35725167" os_list="linux,windows" file_path="content/browser/loader/resource_dispatcher_host_impl.cc"/>' --> ' <item id="resource_dispatcher_host" hash_code="81157007" type="0" content_hash_code="35725167" os_list="windows" file_path="content/browser/loader/resource_dispatcher_host_impl.cc"/>' That change also made some changes to files that make requests, but not to the actual methods. I don't know how that hash is calculated, but it seems like changing any part of the file that makes requests requires updating that tool, which seems like it's broken.
,
Jan 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/86ff9bc4d831eebf5c0f0b0a04214d1b28e50c3f commit 86ff9bc4d831eebf5c0f0b0a04214d1b28e50c3f Author: rhalavati@google.com <rhalavati@google.com> Date: Thu Jan 11 07:01:55 2018 Activate Network traffic annotation tests for Linux trybot. Traffic annotation tests are updated to run on the changed files instead of the whole repository, which should make tests much faster. Bug: 690323 Bug: 788035 Change-Id: I4eb7c173f5d677cdb9944f02e9c15dfa0a374b10 Reviewed-on: https://chromium-review.googlesource.com/856456 Reviewed-by: Martin Šrámek <msramek@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Cr-Commit-Position: refs/heads/master@{#528573} [modify] https://crrev.com/86ff9bc4d831eebf5c0f0b0a04214d1b28e50c3f/tools/traffic_annotation/README.md [modify] https://crrev.com/86ff9bc4d831eebf5c0f0b0a04214d1b28e50c3f/tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1 [modify] https://crrev.com/86ff9bc4d831eebf5c0f0b0a04214d1b28e50c3f/tools/traffic_annotation/scripts/check_annotations.py
,
Jan 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64c23fad92332d3b445989bbc535ac9d498311b1 commit 64c23fad92332d3b445989bbc535ac9d498311b1 Author: rhalavati@google.com <rhalavati@google.com> Date: Mon Jan 15 09:07:43 2018 Activate network traffic annotation trybot. After updating the traffic annotation and check script in crrev.com/c/863483, network traffic annotation trybot is activated. Bug: 801192 Bug: 788035 Change-Id: I3bdc2bab33445cb8042c773ec5cca4017e8b3027 Reviewed-on: https://chromium-review.googlesource.com/863482 Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Reviewed-by: Martin Šrámek <msramek@chromium.org> Cr-Commit-Position: refs/heads/master@{#529232} [modify] https://crrev.com/64c23fad92332d3b445989bbc535ac9d498311b1/tools/traffic_annotation/scripts/check_annotations.py
,
Jan 16 2018
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e0e6d654fcdae192825324c811798a63dff0ae9 commit 0e0e6d654fcdae192825324c811798a63dff0ae9 Author: John Abd-El-Malek <jam@chromium.org> Date: Tue Jan 30 03:11:20 2018 Revert "Activate Network traffic annotation tests for Linux trybot." This reverts commit 86ff9bc4d831eebf5c0f0b0a04214d1b28e50c3f. Reason for revert: flaky, i.e. see https://ci.chromium.org/buildbot/tryserver.chromium.linux/linux_chromium_rel_ng/634904 https://ci.chromium.org/buildbot/tryserver.chromium.linux/linux_chromium_rel_ng/634934 https://ci.chromium.org/buildbot/tryserver.chromium.linux/linux_chromium_rel_ng/634954 which fail with Returned error text: Traceback (most recent call last): File "/b/c/b/linux/src/tools/clang/scripts/run_tool.py", line 315, in <module> sys.exit(main()) File "/b/c/b/linux/src/tools/clang/scripts/run_tool.py", line 281, in main f.write(compile_db.GenerateWithNinja(args.p)) File "/b/c/b/linux/src/tools/clang/pylib/clang/compile_db.py", line 94, in GenerateWithNinja GetNinjaPath(), '-C', path, '-t', 'compdb', 'cc', 'cxx', 'objc', File "/b/c/b/linux/src/tools/clang/pylib/clang/compile_db.py", line 76, in GetNinjaPath os.path.dirname(os.realpath(__file__)), '..', '..', '..', 'third_party', AttributeError: 'module' object has no attribute 'realpath' Original change's description: > Activate Network traffic annotation tests for Linux trybot. > > Traffic annotation tests are updated to run on the changed files instead > of the whole repository, which should make tests much faster. > > Bug: 690323 > Bug: 788035 > Change-Id: I4eb7c173f5d677cdb9944f02e9c15dfa0a374b10 > Reviewed-on: https://chromium-review.googlesource.com/856456 > Reviewed-by: Martin Šrámek <msramek@chromium.org> > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > Commit-Queue: Ramin Halavati <rhalavati@chromium.org> > Cr-Commit-Position: refs/heads/master@{#528573} TBR=dpranke@chromium.org,msramek@chromium.org,rhalavati@chromium.org Bug: 690323 , 788035 Change-Id: I048be491eb64c221bb4edeab2e0d2aa051b5faa7 Reviewed-on: https://chromium-review.googlesource.com/892383 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#532741} [modify] https://crrev.com/0e0e6d654fcdae192825324c811798a63dff0ae9/tools/traffic_annotation/scripts/check_annotations.py
,
Jul 19
Have you ever considered implementing these checks as a clang plugin, not as a tool? That way check results could be cached by compiler caching systems and checks would be almost instant on all platforms. It could be a separate argument to clang plugin to enable annotation checks that would be passed only if according gn arg is enabled (to make sure these checks can be disabled to have current behaviour).
,
Jul 19
That would be a good idea, but it was not done back then as clang plugins where considered much harder to maintain. Currently the test speed is increased by reducing it to minimal number of files. |
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Nov 23 2017