Skip to content
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.

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

[release/8.0][wasm] Fix perf pipeline runs #93888

Merged
merged 4 commits into from Oct 24, 2023

Conversation

radical
Copy link
Member

@radical radical commented Oct 23, 2023

CI only change:

  1. Remove --experimental-wasm-eh argument from the wasm_args used for wasm performance runs. ([PERF] Remove --experimental-wasm-eh argument from used wasm_args #93357)
    (cherry picked from commit a770017fea3549e0bf88f7c619b79a731271e305)

  2. performance-setup.*: Use release/8.0 as the default channel

  3. Fix passing wasmArgs for bdn

Fixes #93880 .

@radical radical added arch-wasm WebAssembly architecture area-Infrastructure-mono perf-pipeline Issues with dotnet-runtime-perf, or runtime-wasm-perf pipelines labels Oct 23, 2023
@ghost
Copy link

ghost commented Oct 23, 2023

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

Remove --experimental-wasm-eh argument from the wasm_args used for wasm performance runs. (#93357)

(cherry picked from commit a770017)

Author: radical
Assignees: -
Labels:

arch-wasm, area-Infrastructure-mono, perf-pipeline

Milestone: -

@ghost ghost assigned radical Oct 23, 2023
@radical
Copy link
Member Author

radical commented Oct 23, 2023

@radical
Copy link
Member Author

radical commented Oct 23, 2023

Next failure: RuntimeError: Unable to determine the .NET SDK used for net9.0
Working on it.

@LoopedBard3
Copy link
Member

Next failure: RuntimeError: Unable to determine the .NET SDK used for net9.0 Working on it.

I think the error is being hit because the performance-setup.sh script is telling the ci_setup to use the main channel when it should be using the 8.0 channel. https://github.com/dotnet/runtime/blob/main/eng/testing/performance/performance-setup.sh#L380-L385. If we change the default on the release/8.0 branch to be 8.0, any testing going forward will work. The reason this is not currently breaking 8.0 is because by default, the release/* branches use whatever is in place of the * for the ci_setup channel.

@radical
Copy link
Member Author

radical commented Oct 23, 2023

Next failure: RuntimeError: Unable to determine the .NET SDK used for net9.0 Working on it.

I think the error is being hit because the performance-setup.sh script is telling the ci_setup to use the main channel when it should be using the 8.0 channel. main/eng/testing/performance/performance-setup.sh#L380-L385. If we change the default on the release/8.0 branch to be 8.0, any testing going forward will work. The reason this is not currently breaking 8.0 is because by default, the release/* branches use whatever is in place of the * for the ci_setup channel.

You are correct. On my test PR, performance-setup.sh gets branch=refs/heads/wasm-perf-fix-8 and thus picks up the default. I'll change the default as you suggested.

@LoopedBard3
Copy link
Member

Next failure: RuntimeError: Unable to determine the .NET SDK used for net9.0 Working on it.

I think the error is being hit because the performance-setup.sh script is telling the ci_setup to use the main channel when it should be using the 8.0 channel. main/eng/testing/performance/performance-setup.sh#L380-L385. If we change the default on the release/8.0 branch to be 8.0, any testing going forward will work. The reason this is not currently breaking 8.0 is because by default, the release/* branches use whatever is in place of the * for the ci_setup channel.

You are correct. On my test PR, performance-setup.sh gets branch=refs/heads/wasm-perf-fix-8 and thus picks up the default. I'll change the default as you suggested.

We should probably also update the default in the performance-setup.ps1 file as well.

@radical
Copy link
Member Author

radical commented Oct 23, 2023

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tell-mode, so approved.
Is it intended to have it pointing release/8.0?

@carlossanlop carlossanlop added the Servicing-approved Approved for servicing release label Oct 23, 2023
@radical
Copy link
Member Author

radical commented Oct 23, 2023

Tell-mode, so approved. Is it intended to have it pointing release/8.0?

Yes. the test build is still running though, so don't merge yet as there might be additional fixes needed.

@radical radical changed the title [releaes/8.0][wasm] Fix perf pipeline runs [release/8.0][wasm] Fix perf pipeline runs Oct 23, 2023
@radical
Copy link
Member Author

radical commented Oct 23, 2023

Wasm run failed :

[2023/10/23 14:39:19][INFO] ERROR(S):
[2023/10/23 14:39:19][INFO]   Option 'expose_wasm --module' is unknown.

.. with command line containing --wasmArgs "--expose_wasm --module". Pushed a fix for that, and started a new wasm only run - https://dev.azure.com/dnceng/internal/_build/results?buildId=2299261&view=results

@radical
Copy link
Member Author

radical commented Oct 24, 2023

started a new wasm only run - dev.azure.com/dnceng/internal/_build/results?buildId=2299261&view=results

No helix jobs run yet. It's been waiting for 2.5hours now.

@radical
Copy link
Member Author

radical commented Oct 24, 2023

/azp run runtime-wasm-perf

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@carlossanlop
Copy link
Member

This is green @radical. Let me know if it's ready to merge.

@radical
Copy link
Member Author

radical commented Oct 24, 2023

This is green @radical. Let me know if it's ready to merge.

The CI job is still stuck and hasn't run any of the helix jobs. The changes seem to be fine locally. This can only improve the current completely broken state! Please go ahead and merge this!

@carlossanlop carlossanlop merged commit 488a8a3 into dotnet:release/8.0 Oct 24, 2023
178 checks passed
@radical radical deleted the wasm-perf-fix-8 branch October 24, 2023 02:34
@radical
Copy link
Member Author

radical commented Oct 24, 2023

Update: the pipeline is running successfully.

lewing added a commit that referenced this pull request Oct 28, 2023
* [release/8.0] Stable branding for .NET 8 GA

* Handle an empty bandPreleaseVersion

* [release/8.0] Update dependencies from dotnet/emsdk (#93801)

* Update dependencies from https://github.com/dotnet/emsdk build 20231020.2

Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100.Transport
 From Version 8.0.0-rtm.23511.3 -> To Version 8.0.0-rtm.23520.2

* switch to the stable version now

* Update dependencies

* Also fix the trigger

* pin the wbt sdk to avoid the latest analizer nonsense

* Use source generation for the serializer

* Try to make sourcebuild happy

* Try again to make sourcebuild happy

* Use reflection and suppress trim analysis instead

This reverts commit 768b65b.

* Fix reverting too much

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Larry Ewing <lewing@microsoft.com>

* [release/8.0] Update APICompat settings under source build (#93865)

* Update APICompat settings under source build

* Update resolveContract.targets

---------

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

* Override InformationalVersion for NativeAOT corelib too

* [release/8.0] Improve performance of UnmanagedMemoryStream (#93812)

* Improve performance of UnmanagedMemoryStream

UnmanagedMemoryStream used Interlocked operations to update its state to prevent tearing of 64-bit values on 32-bit platforms. This pattern is expensive in general and it was found to be prohibitively expensive on recent hardware..

This change removes the expensive Interlocked operations and addresses
the tearing issues in alternative way:
- The _length field is converted to nuint that is guaranteed to be
  updated atomically.
- Writes to _length field are volatile to guaranteed the
  unininitialized memory cannot be read.
- The _position field remains long and it has a risk of tearing. It is
  not a problem since tearing of this field cannot lead to buffer
  overruns.

Fixes #93624

* Add comment

---------

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

* Update dependencies from https://github.com/dotnet/emsdk build 20231023.2 (#93881)

Microsoft.SourceBuild.Intermediate.emsdk , Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100
 From Version 8.0.0-rtm.23520.2 -> To Version 8.0.0-rtm.23523.2

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* [release/8.0] Update dependencies from dnceng/internal/dotnet-optimization (#93827)

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20231021.3

optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime , optimization.PGO.CoreCLR
 From Version 1.0.0-prerelease.23519.5 -> To Version 1.0.0-prerelease.23521.3

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20231021.3

optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime , optimization.PGO.CoreCLR
 From Version 1.0.0-prerelease.23519.5 -> To Version 1.0.0-prerelease.23521.3

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* [release/8.0][wasm] Fix perf pipeline runs (#93888)

* Remove --experimental-wasm-eh argument from the wasm_args used for wasm performance runs. (#93357)

(cherry picked from commit a770017)

* performance-setup.sh: Use `release/8.0` as the default channel

* performance-setup.ps1: use release/8.0 as the default channel

* Fix passing wasmArgs for bdn

---------

Co-authored-by: Parker Bibus <parkerbibus@microsoft.com>

* [release/8.0] Honor JsonSerializerOptions.PropertyNameCaseInsensitive in property name conflict resolution. (#93935)

* Honor JsonSerializerOptions.PropertyNameCaseInsensitive in property name conflict resolution.

* Update src/libraries/System.Text.Json/tests/Common/PropertyNameTests.cs

Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>

* Address feedback

---------

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>

* [8.0] Update MsQuic (#93979)

* Try pinning the installer version to a 8.01xx sdk

* Target net8.0 in SatelliteAssemblyFromProjectRef

---------

Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>
Co-authored-by: Larry Ewing <lewing@microsoft.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Parker Bibus <parkerbibus@microsoft.com>
Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
@dotnet dotnet locked as resolved and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Infrastructure-mono perf-pipeline Issues with dotnet-runtime-perf, or runtime-wasm-perf pipelines Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants