8000 [Windows Arm64] Benchmarks incorrectly bucketed as 'intel' architecture · Issue #135722 · flutter/flutter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Windows Arm64] Benchmarks incorrectly bucketed as 'intel' architecture #135722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
1 task done
loic-sharma opened this issue Sep 28, 2023 · 7 comments
Closed
1 task done

[Windows Arm64] Benchmarks incorrectly bucketed as 'intel' architecture #135722

loic-sharma opened this issue Sep 28, 2023 · 7 comments
Labels
infra: device lab Infrastructure device lab problems P2 Important issues not at the top of the work list team-infra Owned by Infrastructure team triaged-infra Triaged by Infrastructure team

Comments

@loic-sharma
Copy link
Member
loic-sharma commented Sep 28, 2023 •
edited
Loading

Is there an existing issue for this?

Type of Request

bug

Infrastructure Environment

LUCI

What is happening?

The Windows Arm64 benchmarks are incorrectly bucketed with the Windows x64 benchmarks. As a result, it appears that Windows x64 benchmarks have massively regressed.

The recipes set the arch benchmark tag to intel on Windows Arm64. In this build example, notice how --benchmark-tags has "arch": "intel" even though PROCESSOR_IDENTIFIER: ARMv8 (64-bit) Family 8 Model D4B Revision 0, Qualcomm Technologies Inc

It looks like the root cause is that the Windows Arm64 Python installation returns AMD64 for platform.machine(). As a result, LUCI returns intel for the architecture. The Flutter recipes likely need to workaround this Python bug.

Steps to reproduce

Step 1: Open a Windows Arm64 benchmark: https://ci.chromium.org/ui/p/flutter/builders/staging/Windows_arm64%20complex_layout_win_desktop__start_up
Step 2: Open the stdout for the upload results step
Step 3: Check the arch benchmark tag. The actual value is intel, which is incorrect.

Expected results

The arch benchmark tag should be arm.
@loic-sharma loic-sharma added the team-infra Owned by Infrastructure team label Sep 28, 2023
@loic-sharma loic-sharma mentioned this issue Sep 28, 2023
Closed
@loic-sharma loic-sharma changed the title [Windows Arm64] Benchmarks incorrect report 'arch' is 'intel' [Windows Arm64] Benchmarks incorrectly bucketed as 'intel' architecture Sep 28, 2023
@loic-sharma loic-sharma mentioned this issue Sep 28, 2023
Merged
8 tasks
@keyonghan
Copy link
Contributor

@yusuf-goog Could you have take a look at the python installed on these bots via salt? We should install an arm64 versioned python on these arm machines.

@keyonghan keyonghan added the infra: device lab Infrastructure device lab problems label Sep 28, 2023
@cbracken
Copy link
Member
cbracken commented Sep 28, 2023 •
edited
Loading

If it's going to take a while to get an arm64 python build, is there a quick workaround?

For macOS we use something like this, which is unfortunately very BSD-specific:
https://github.com/flutter/engine/blob/2052515c44f3696a5440888bf5a773e5aa8b4f2e/tools/gn#L147-L156

Looks like the Dart SDK uses this for Windows:
https://github.com/dart-lang/sdk/blob/baaf5e1116104e24673399d1fd0b772de596e166/tools/utils.py#L258-L263

Sadly that only helps at build time, so doesn't help us during devicelab testing unless that happens to be set in our luci recipes (do we use cipd for devicelab?) :(

Looks like Windows has a %PROCESSOR_ARCHITECTURE% environment variable?

@loic-sharma loic-sharma mentioned this issue Sep 28, 2023
Closed
8 tasks
@keyonghan
Copy link
Contributor

Looks like Windows has a %PROCESSOR_ARCHITECTURE% environment variable?

Unfortunately this returns the same based on the x64 python:

>>> os.environ.get("PROCESSOR_ARCHITECTURE") 
'AMD64'

One way we can do is explicitly provide the arch as a property, which then be retrieved from recipes.

@keyonghan keyonghan mentioned this issue Sep 28, 2023
Merged
@keyonghan
Copy link
Contributor

#135725 to add the property to windows_arm64 platform.
https://flutter-review.googlesource.com/c/recipes/+/51420 to support the property from recipes.

After these two pr/cl land, this should be unblocked. Then the remaining action item here is to install arm64 python instead of x64 version on these arm windows testbeds.

auto-submit bot pushed a commit that referenced this issue Sep 29, 2023
@keyonghan
Add arch property for windows_arm64 platform (#135725)
951653a 
Window arm64 bots are using x64 built python, which gave incorrect arch info. This PR adds the arch info explicitly so that recipes can get this value and populate to benchmark tags.

Part of #135722
@pbo-linaro
Copy link
Contributor
pbo-linaro commented Sep 29, 2023 •
edited
Loading

We wrote some documentation of PROCESSOR_ARCHITECTURE env var here.

Since this value is set depending on binary architecture, there is no easy way to change it. You need to run a native windows-arm64 python.

Setting the property manually like it was proposed is probably the best/cleanest solution.

@yusuf-goog
Copy link
Contributor

platform.processor() should get you whats needed?

Mairramer pushed a commit to Mairramer/flutter that referenced this issue Oct 10, 2023
@keyonghan @Mairramer
Add arch property for windows_arm64 platform (flutter#135725)
3bfe74c 
Window arm64 bots are using x64 built python, which gave incorrect arch info. This PR adds the arch info explicitly so that recipes can get this value and populate to benchmark tags.

Part of flutter#135722
@keyonghan keyonghan added triaged-infra Triaged by Infrastructure team P2 Important issues not at the top of the work list labels Oct 12, 2023
cbracken pushed a commit to cbracken/flutter_recipes that referenced this issue May 29, 2024
@keyonghan
Add arch property option
df0f4d8 
Change-Id: Id7fa934f8dbea0461b8dbe39c3575e72d5f9e02b
Bug: flutter/flutter#135722
Reviewed-on: https://flutter-review.googlesource.com/c/recipes/+/51420
Reviewed-by: Loïc Sharma <loicsharma@google.com>
Commit-Queue: Keyong Han <keyonghan@google.com>
@matanlurey matanlurey closed this as not planned Won't fix, can't repro, duplicate, stale Feb 22, 2025
@github-actions GitHub Actions
Copy link
github-actions bot commented Mar 8, 2025

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
infra: device lab Infrastructure device lab problems P2 Important issues not at the top of the work list team-infra Owned by Infrastructure team triaged-infra Triaged by Infrastructure team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@matanlurey @cbracken @loic-sharma @keyonghan @yusuf-goog @pbo-linaro
0