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

[mono][tests] Parallel AOT compilation of runtime tests on iOS platforms #86089

Merged
merged 90 commits into from
Jul 21, 2023

Conversation

kotlarmilos
Copy link
Member

@kotlarmilos kotlarmilos commented May 11, 2023

Description

This PR aims to move the AOT compilation of runtime tests to the Helix and enable the following testing groups on apple mobile platforms: tracing_eventpipe, tracing_eventlistener, reflection_regression, reflection_StaticInterfaceMembers, reflection_GenericAttribute, reflection_DefaultInterfaceMethods, baseservices_mono, baseservices_exceptions, baseservices_compilerservices, baseservices_TieredCompilation, Regressions_coreclr, Loader_classloader, Loader_StartupHooks, JIT_RyuJIT, JIT_PGO, JIT_Math, JIT_Intrinsics, Interop_DllImportSearchPaths, JIT_Intrinsics, GC_Scenarios, GC_Regressions, GC_LargeMemor, GC_Features, GC_Coverage, GC_API, Exceptions_UnwindFpBleedTest, CoreMangLib_system.

The Helix payload is generated using the runtime pipeline. Then, the libraries pipeline is utilized to AOT compile and execute the tests on Helix. The build-runtime-tests-and-send-to-helix.yml is updated with the compileOnHelix parameter, which if set, uses the libraries/helix.yml template instead of the runtime send-to-helix-step.yml template. The runtime and libraries pipelines are updated to support AOT compilation of runtime tests on Helix.

Important notes:

@kotlarmilos kotlarmilos added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it area-Infrastructure-mono os-ios Apple iOS labels May 11, 2023
@kotlarmilos kotlarmilos added this to the 8.0.0 milestone May 11, 2023
@kotlarmilos kotlarmilos self-assigned this May 11, 2023
@ghost
Copy link

ghost commented May 11, 2023

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

Issue Details

This PR aims to move AOT compilation of the runtime tests to Helix.

Author: kotlarmilos
Assignees: kotlarmilos
Labels:

NO-MERGE, NO-REVIEW, area-Infrastructure-mono, os-ios

Milestone: 8.0.0

@kotlarmilos
Copy link
Member Author

@steveisok @akoeplinger @ivanpovazan @mdh1418 This pull request is ready for review. I am triaging failing tests by fixing or disabling them, but it should not affect the functionality.

List of follow-up tasks is in #84254.

@SamMonoRT Currently, there are 28 test suites (apps) that can be run on iOS, out of which 7 are failing. I will try to fix or disable the failing tests and run the remaining ones. Afterward, I suggest we review other disabled test suites for AOT compilation and prioritize those with higher impact.

@kotlarmilos
Copy link
Member Author

/azp run runtime-ioslike,runtime-ioslikesimulator

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 86089 in repo dotnet/runtime

@kotlarmilos
Copy link
Member Author

/azp run runtime-ioslike,runtime-ioslikesimulator

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@kotlarmilos
Copy link
Member Author

kotlarmilos commented Jul 20, 2023

/azp run runtime-ioslikesimulator

@azure-pipelines

This comment was marked as outdated.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kotlarmilos
Copy link
Member Author

/azp run runtime-ioslikesimulator

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 86089 in repo dotnet/runtime

@kotlarmilos
Copy link
Member Author

/azp run runtime-ioslikesimulator

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kotlarmilos
Copy link
Member Author

ping here :)

@steveisok @akoeplinger @ivanpovazan @mdh1418

Copy link
Member

@ivanpovazan ivanpovazan left a comment

Choose a reason for hiding this comment

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

Apart from a few comments/questions looks good to me!

Comment on lines 65 to 73
<!--
Checks if the directory name "AppleAssembliesToBundle" is equal to the "AssemblyName",
if they match, consider it as the root directory and copy the files there,
otherwise, copy the files to a subdirectory.
-->
<BundleFiles Condition="'%(AppleAssembliesToBundle._IsNative)' != 'true' and ($([System.IO.Path]::GetFileName($([System.IO.Path]::GetDirectoryName('%(AppleAssembliesToBundle.Identity)')))) == '$(AssemblyName)' or '$(CustomMain)' != 'true')"
Include="@(AppleAssembliesToBundle)" TargetDir="publish" />
<BundleFiles Condition="'%(AppleAssembliesToBundle._IsNative)' != 'true' and $([System.IO.Path]::GetFileName($([System.IO.Path]::GetDirectoryName('%(AppleAssembliesToBundle.Identity)')))) != '$(AssemblyName)' and '$(CustomMain)' == 'true'"
Include="@(AppleAssembliesToBundle)" TargetDir="publish\$([System.IO.Path]::GetFileName($([System.IO.Path]::GetDirectoryName('%(AppleAssembliesToBundle.Identity)'))))" />
Copy link
Member

Choose a reason for hiding this comment

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

Just a question: Why does it matter if $(CustomMain) is specified or not here?

Copy link
Member Author

@kotlarmilos kotlarmilos Jul 21, 2023

Choose a reason for hiding this comment

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

The same target is used for the library and runtime tests and the condition checks if the directory is equals to the assemblyName. In case of runtime tests that is true, but for library tests that creates recursive dirs.

Probably better to use more descriptive variable.

src/mono/msbuild/apple/build/AppleBuild.props Show resolved Hide resolved
src/tests/build.proj Show resolved Hide resolved
<Copy SourceFiles="@(RecursiveCopyHack)" DestinationFolder="$(FinalPath)/%(RecursiveDir)" />
<RemoveDir Directories="$(AppBundlePath)" />
<Message Importance="High" Text="App: $(FinalPath)" />
<CustomTask EntryPoints="@(EntryPoints)" RunScriptCommand="$(RunScriptCommand)" AppName="$(Category)" Condition="'$(BuildTestsOnHelix)' == 'true'">
Copy link
Member

Choose a reason for hiding this comment

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

nit: I don't have a good suggestion, but I think the task should have a more meaningful name.
Maybe something like UpdateRunScriptToHandleMultipleEntryPoints

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course, I missed that.

@kotlarmilos
Copy link
Member Author

Merging this PR to enhance the apple mobile tests coverage. If additional feedback is provided, it will be addressed in a follow-up.

@kotlarmilos kotlarmilos merged commit 7531c50 into dotnet:main Jul 21, 2023
168 of 171 checks passed
@steveisok
Copy link
Member

LGTM! Awesome work!

@ghost ghost locked as resolved and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants