New issue
Advanced search Search tips

Issue 910699 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocked on:
issue 899438
issue angleproject:2996

Blocking:
issue 314403
issue 330263



Sign in to add a comment

"Deterministic Linux" broken after vulkan roll => VkLayer_*.json generation step has underspecified dependencies

Project Member Reported by thakis@chromium.org, Nov 30

Issue description

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Deterministic%20Linux/18213

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8928406207145120128/+/steps/compare_build_artifacts/0/stdout

angledata/VkLayer_core_validation.json                                                                                                                           : DIFFERENT (unexpected): 1 out of 1317 bytes are different (0.08%)
  0xc0      : 2020202020202020226170695f76657273696f6e223a2022312e312e3835222c '        "api_version": "1.1.85",'
              2020202020202020226170695f76657273696f6e223a2022312e312e3837222c '        "api_version": "1.1.87",'
                                                                         ^                                   ^
angledata/VkLayer_object_tracker.json                                                                                                                            : DIFFERENT (unexpected): 1 out of 939 bytes are different (0.11%)
  0xc0      : 202020202020226170695f76657273696f6e223a2022312e312e3835222c0a20 '      "api_version": "1.1.85",. '
              202020202020226170695f76657273696f6e223a2022312e312e3837222c0a20 '      "api_version": "1.1.87",. '
                                                                     ^                                     ^
angledata/VkLayer_parameter_validation.json                                                                                                                      : DIFFERENT (unexpected): 1 out of 951 bytes are different (0.11%)
  0xe0      : 2022312e312e3835222c0a202020202020202022696d706c656d656e74617469 ' "1.1.85",.        "implementati'
              2022312e312e3837222c0a202020202020202022696d706c656d656e74617469 ' "1.1.87",.        "implementati'
                             ^                                                         ^
angledata/VkLayer_standard_validation.json                                                                                                                       : DIFFERENT (unexpected): 1 out of 538 bytes are different (0.19%)
  0xa0      : 312e3835222c0a202020202020202022696d706c656d656e746174696f6e5f76 '1.85",.        "implementation_v'
              312e3837222c0a202020202020202022696d706c656d656e746174696f6e5f76 '1.87",.        "implementation_v'
                     ^                                                             ^
angledata/VkLayer_threading.json                                                                                                                                 : DIFFERENT (unexpected): 1 out of 469 bytes are different (0.21%)
  0xc0      : 5f76657273696f6e223a2022312e312e3835222c0a202020202020202022696d '_version": "1.1.85",.        "im'
              5f76657273696f6e223a2022312e312e3837222c0a202020202020202022696d '_version": "1.1.87",.        "im'
                                                 ^                                               ^
angledata/VkLayer_unique_objects.json                                                                                                                            : DIFFERENT (unexpected): 1 out of 319 bytes are different (0.31%)
  0xc0      : 202020202020226170695f76657273696f6e223a2022312e312e3835222c0a20 '      "api_version": "1.1.85",. '
              202020202020226170695f76657273696f6e223a2022312e312e3837222c0a20 '      "api_version": "1.1.87",. '
                                                                     ^                                     ^


The bot has one incremental build dir and one clobber build dir. It looks like these json files don't get rebuilt on vulkan rolls, so they don't get updated on the incremental bot.

The fix is probably to put the api_version on the commandline of the command that updates them so that ninja's commandline tracking will auto-regen them on version bumps.
 
Looks like https://cs.chromium.org/chromium/src/third_party/angle/third_party/vulkan-validation-layers/BUILD.gn?q=VkLayer_parameter_validation&sq=package:chromium&g=0&l=328 generates these.

It does this: https://cs.chromium.org/chromium/src/third_party/angle/scripts/generate_vulkan_layers_json.py?q=generate_vulkan_layers_json.py&sq=package:chromium&g=0&l=1

# Get the Vulkan version from the vulkan_core.h file
vk_header_filename = os.path.join(angle_root_dir, "third_party", "vulkan-headers", "src", "include", "vulkan", "vulkan_core.h")
vk_version = '83'
with open(vk_header_filename) as vk_header_file:
    for line in vk_header_file:
        if line.startswith("#define VK_HEADER_VERSION"):
            vk_version = line.split()[-1]
            break

But it doesn't have a dependency on that header file, so it doesn't rerun when the header changes. Easiest is to add that header to the 'sources' list in that gn file, that should be sufficient.
The script also does globbing: https://cs.chromium.org/chromium/src/third_party/angle/scripts/generate_vulkan_layers_json.py?q=generate_vulkan_layers_json.py&sq=package:chromium&g=0&l=68

I'd recommend instead passing the .in files listed in gn to the script (https://cs.chromium.org/chromium/src/third_party/angle/third_party/vulkan-validation-layers/BUILD.gn?q=VkLayer_parameter_validation&sq=package:chromium&g=0&l=329), then there's no way that the .in lists in gn and the .in files in the .py script get out of sync.
Blockedon: 899438
(found by the work in issue 899438)
Summary: "Deterministic Linux" broken after vulkan roll => VkLayer_*.json generation step has underspecified dependencies (was: "Deterministic Linux" broken after vulkan roll)
Labels: Sheriff-Chromium
+Sheriff-Chromium
Owner: thakis@chromium.org
Status: Started (was: Untriaged)
Looks like the bot is still red, let me give this a try...
OK, thanks Nico. I just got in. If you need help you can ping me on chat.
I think I have an idea what I want to do, but I have a question on how to organize it.

Currently, generate_vulkan_layers_json.py is in the angle repo and used from the vulkan-validation-layers and vulkan-tools repos. I want to change generate_vulkan_layers_json.py command-line interface.

Is the following approach ok?

1. Update vulkan-validation-layers vulkan-tools to use new interface (which doesn't yet exist, but since angle still pins the old revision that's maybe fine)
2. In one CL, land patch to generate_vulkan_layers_json.py and roll vulkan-tools, vulkan-validation-layers

Also, generate_vulkan_layers_json.py does two more or less completely distinct things, one thing if called from the one place, another if called from the other. Alternatively, I could do

1. Add a small script doing the one thing in vulkan-tools and update the BUILD.gn file there to call that
2. Add a small script doing the other thing in vulkan-validation-layers and update the BUILD.gn file there to call that
3. Remove now-unused generate_vulkan_layers_json.py from angle and roll deps

Do you have a preference for which approach to take?
Oh wait, looks like the two .gn files are in the angle repo too, despite being in third_party/foo subdirs. So it's all in one repo, which makes landing the patch easy.

I'll just send you what I have, but splitting the script into two might still make sense -- you can tell me to do that on the review :-)
Labels: -Sheriff-Chromium
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/ce0a8f3c8fa7025b5836d80745068484a457cb4e

commit ce0a8f3c8fa7025b5836d80745068484a457cb4e
Author: Nico Weber <thakis@chromium.org>
Date: Mon Dec 03 15:48:33 2018

List vulkan_core.h as input of generate_vulkan_layers_json.py.

Since this was missing, the layer json files didn't get regenerated on
vulkan rolls, leading to stale generated json files, which in turn led
to incremental builds having different files in the swarming isolate
than full builds.

To make this type of bug harder to introduce, rewrite
generate_vulkan_layers_json.py a bit:

- pass in path to vulkan_core.h as an argument
- also pass in the input .json / .json.in files as arguments,
  so that the script re-runs if a .json or .json.in input is added
  or removed, and in the script verify that the passed-in list matches
  the glob() the script did previously (this verifies that the sources
  list in the .gn file is up-to-date with the state on disk)
- generate outputs list in gn from sources list, to make sure they're
  in sync
- use an expicit --icd flag instead of doing `'icd' in path`
- fail when failing to extract vk_version instead of silently using a
  default
- some minor python style fixes

Bug:  chromium:910699 ,chromium:869348
Change-Id: I1e598f4566697a7f1ef56b040e52d0717f7ad075
Reviewed-on: https://chromium-review.googlesource.com/c/1358631
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>

[modify] https://crrev.com/ce0a8f3c8fa7025b5836d80745068484a457cb4e/scripts/generate_vulkan_layers_json.py
[modify] https://crrev.com/ce0a8f3c8fa7025b5836d80745068484a457cb4e/third_party/vulkan-tools/BUILD.gn
[modify] https://crrev.com/ce0a8f3c8fa7025b5836d80745068484a457cb4e/third_party/vulkan-validation-layers/BUILD.gn

Blockedon: angleproject:2996
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b45b2eaa206f2d443e7a58703c4bcbcba837a876

commit b45b2eaa206f2d443e7a58703c4bcbcba837a876
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Mon Dec 03 18:39:55 2018

Roll src/third_party/angle 317a9ebdb019..c10a023a66f2 (6 commits)

https://chromium.googlesource.com/angle/angle.git/+log/317a9ebdb019..c10a023a66f2


git log 317a9ebdb019..c10a023a66f2 --date=short --no-merges --format='%ad %ae %s'
2018-12-03 jmadill@chromium.org Fix fuchsia build of libfeature_support.
2018-12-03 thakis@chromium.org List vulkan_core.h as input of generate_vulkan_layers_json.py.
2018-12-03 Tom.Tan@microsoft.com Don't copy d3dcompiler_47.dll when build target is Windows ARM64
2018-12-03 jmadill@chromium.org Refactor test shader style.
2018-11-30 syoussefi@chromium.org Vulkan: Add dynamic index buffers to graph
2018-11-30 ianelliott@google.com Version-2 API of the A4A opt-in/out (a.k.a. feature-support utilities)


Created with:
  gclient setdep -r src/third_party/angle@c10a023a66f2

The AutoRoll server is located here: https://autoroll.skia.org/r/angle-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:910699 ,chromium:869348,chromium:893460,chromium:b/113346561
TBR=ynovikov@chromium.org

Change-Id: I29ee936a536269990c413cdd50ad7d46601914a6
Reviewed-on: https://chromium-review.googlesource.com/c/1358758
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#613170}
[modify] https://crrev.com/b45b2eaa206f2d443e7a58703c4bcbcba837a876/DEPS

Sign in to add a comment