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

Add New Test to Track Number of Jitted Methods #91235

Merged
merged 24 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7b31090
New beginning for the Jitted Methods Counting Test.
ivdiazsa Aug 28, 2023
b565e74
EXPERIMENTAL CHANGE ONLY: Added a command to print the whole directory
ivdiazsa Aug 29, 2023
2317a4b
experimental change only: Discovered the jobs have dependencies on
ivdiazsa Aug 29, 2023
caf11e8
EXPERIMENT-ONLY CHANGE: Added the 'ls -R' directly to the Helix Post
ivdiazsa Aug 30, 2023
fda5625
Merge branch 'main' into jitted-parts
ivdiazsa Aug 30, 2023
efd048d
Maybe it doesn't like me removing the innerloop...?
ivdiazsa Aug 30, 2023
d243aef
More experimenting... So we can't do just musl...
ivdiazsa Aug 30, 2023
90b639e
Merge branch 'main' into jitted-parts
ivdiazsa Aug 31, 2023
87212da
Changed the test's methodology to use the JitInfo API, instead of
ivdiazsa Aug 31, 2023
7c3cca8
Added the `AlwaysUseCrossgen2` property to the HelloWorld test app.
ivdiazsa Sep 5, 2023
85f3317
Now printing the Jitted Methods to compare with my local machine.
ivdiazsa Sep 6, 2023
0893d48
REVIEW-READY: Reenabled all the pipelines to see if there are any
ivdiazsa Sep 7, 2023
35bc35e
Changed the Jitted methods counter to consider the whole process,
ivdiazsa Sep 7, 2023
cf7996b
DEBUG-ONLY CHANGE: We're getting more Jitted methods on Arm64 for some
ivdiazsa Sep 8, 2023
bb026de
Merge branch 'main' into jitted-parts
ivdiazsa Sep 8, 2023
1a33f0b
Enabled musl outerloop pipelines because we were observing different
ivdiazsa Sep 8, 2023
88d59f2
Will you stop complaining now?
ivdiazsa Sep 8, 2023
ad3f2fd
READY: Reenabled the pipelines and disabled the test for R2R_CG2
ivdiazsa Sep 11, 2023
b99398d
Changed the jits-number limit.
ivdiazsa Sep 11, 2023
adbb0b2
Changed the test to not require an external app anymore, and fixed
ivdiazsa Sep 12, 2023
1e81e8a
DEBUG-ONLY CHANGE: Printing some logging to try to figure out why
ivdiazsa Sep 18, 2023
ad7a855
...
ivdiazsa Sep 18, 2023
376b205
...
ivdiazsa Sep 18, 2023
d9cb394
Due to how some machines are set up, it is actually possible for a
ivdiazsa Sep 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eng/pipelines/coreclr/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ extends:
platforms:
- linux_arm
- linux_musl_arm64
- linux_arm64
- linux_x64
- osx_arm64
- osx_x64
Expand Down
47 changes: 5 additions & 42 deletions eng/pipelines/runtime.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ extends:
- ${{ if eq(variables.dependOnEvaluatePaths, true) }}:
- template: /eng/pipelines/common/evaluate-default-paths.yml

# WIP-ONLY CHANGE: Need to "debug" something that only happens in CI, so
# disabling these pipelines to speed up the running procedure.

#
# Build CoreCLR checked
# Only when CoreCLR is changed
Expand All @@ -70,6 +73,7 @@ extends:
jobTemplate: /eng/pipelines/coreclr/templates/build-job.yml
buildConfig: checked
platforms:
- linux_x86
- linux_x64
- linux_arm
- linux_arm64
Expand All @@ -78,6 +82,7 @@ extends:
- linux_musl_arm64
- linux_musl_x64
- osx_arm64
- tizen_armel
- windows_x86
- windows_x64
- windows_arm64
Expand Down Expand Up @@ -186,48 +191,6 @@ extends:
eq(dependencies.evaluate_paths.outputs['SetPathVars_coreclr_jit.containsChange'], true),
eq(variables['isRollingBuild'], true)))

#
# Build CoreCLR with no R2R
#
- template: /eng/pipelines/common/platform-matrix.yml
parameters:
jobTemplate: /eng/pipelines/common/global-build-job.yml
helixQueuesTemplate: /eng/pipelines/libraries/helix-queues-setup.yml
buildConfig: checked
runtimeFlavor: coreclr
platforms:
- linux_x86
jobParameters:
testScope: innerloop
nameSuffix: CoreCLR_NoR2R
buildArgs: -s clr.runtime+clr.jit+clr.iltools+clr.spmi+clr.corelib -c $(_BuildConfig)
timeoutInMinutes: 120
condition: >-
or(
eq(dependencies.evaluate_paths.outputs['SetPathVars_coreclr.containsChange'], true),
eq(variables['isRollingBuild'], true))

#
# Build CoreCLR as a non-portable build
#
- template: /eng/pipelines/common/platform-matrix.yml
parameters:
jobTemplate: /eng/pipelines/common/global-build-job.yml
helixQueuesTemplate: /eng/pipelines/libraries/helix-queues-setup.yml
buildConfig: checked
runtimeFlavor: coreclr
platforms:
- tizen_armel
jobParameters:
testScope: innerloop
nameSuffix: CoreCLR_NonPortable
buildArgs: -s clr.native+clr.tools+clr.corelib+clr.nativecorelib+clr.aot+clr.packages -c $(_BuildConfig) /p:PortableBuild=false
timeoutInMinutes: 120
condition: >-
or(
eq(dependencies.evaluate_paths.outputs['SetPathVars_coreclr.containsChange'], true),
eq(variables['isRollingBuild'], true))

#
# CoreCLR NativeAOT debug build and smoke tests
# Only when CoreCLR is changed
Expand Down
3 changes: 2 additions & 1 deletion src/tests/Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@
<Target Name="CheckForTestOutsideOfGroup" BeforeTargets="Build">
<Error Condition="'$(InMergedTestDirectory)' == 'true'
and '$(OutputType)' == 'Exe'
and '$(_CLRTestNeedsToRun)' == 'true'
and '$(RequiresProcessIsolation)' != 'true'
and '$(BuildAsStandalone)' != 'true'
and '$(IsMergedTestRunnerAssembly)' != 'true'"
Expand Down Expand Up @@ -510,7 +511,7 @@
</ItemGroup>
</Target>

<PropertyGroup Condition="'$(Language)' == 'C#' and ('$(BuildAsStandalone)' == 'true' or '$(RequiresProcessIsolation)' == 'true' or '$(InMergedTestDirectory)' == 'true' or '$(IsMergedTestRunnerAssembly)' == 'true')">
<PropertyGroup Condition="'$(Language)' == 'C#' and '$(_CLRTestNeedsToRun)' == 'true' and ('$(BuildAsStandalone)' == 'true' or '$(RequiresProcessIsolation)' == 'true' or '$(InMergedTestDirectory)' == 'true' or '$(IsMergedTestRunnerAssembly)' == 'true')">
<ReferenceXUnitWrapperGenerator Condition="'$(ReferenceXUnitWrapperGenerator)' == ''">true</ReferenceXUnitWrapperGenerator>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

internal class HelloWorld
{
static int Main(string[] args)
{
Console.WriteLine("Hello World!");
// We're fine converting to int here because if we got that many jitted
// methods that would require a long, then that means something is
// extremely wrong. Actually, I'm not even sure it's possible.
int jits = (int) System.Runtime.JitInfo.GetCompiledMethodCount(false);
return jits > 0 ? jits : -1;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>$(NetCoreAppToolCurrent)</TargetFramework>
<CLRTestKind>BuildOnly</CLRTestKind>
<AlwaysUseCrossgen2>true</AlwaysUseCrossgen2>
</PropertyGroup>

<ItemGroup>
<Compile Include="HelloWorld.cs" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.IO;
using System.Linq;
using Xunit;

using Reflection = System.Reflection;
using InteropServices = System.Runtime.InteropServices;

public class JittedMethodsCountingTest
{
private const int MAX_JITTED_METHODS_ACCEPTED = 100;

[Fact]
public static int TestEntryPoint()
{
// If DOTNET_ReadyToRun is disabled, then this test ought to be skipped.
if (!IsReadyToRunEnvSet())
{
Console.WriteLine("\nThis test is only supported in ReadyToRun scenarios."
+ " Skipping...\n");
return 100;
}

if (IsRunCrossgen2Set() && IsRunningOnARM64())
{
Console.WriteLine("\nThis test is currently unsupported on ARM64 when"
+ " RunCrossGen2 is enabled. Skipping...\n");
return 100;
}

string testAppLocation = Path.GetDirectoryName(Reflection
.Assembly
.GetExecutingAssembly()
.Location);
string appName = Path.Combine(testAppLocation, "HelloWorld.dll");

// For adding any new apps for this test, make sure they return a negative
// number when failing.
int appResult = RunHelloWorldApp(appName);
Copy link
Member

Choose a reason for hiding this comment

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

The setup with the app running in a separate process made sense when the test used JIT logging to get the method count.

This setup is overkill now that the test just calls GetCompiledMethodCount and checks its value. Would it be better to turn this into an ordinary single process test that just calls GetCompiledMethodCount and checks its value (and skipping the check in specific environments)?

Copy link
Contributor Author

@ivdiazsa ivdiazsa Sep 12, 2023

Choose a reason for hiding this comment

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

Actually, this makes a lot of sense. I just tested it on my local machine and I'm getting 4 jitted methods instead of the 1 I used to get with the solo HelloWorld app. Since the number of jitted methods is very similar, does it make sense to you to do it like this instead @trylek?


if (appResult < 0)
{
Console.WriteLine("App failed somewhere and so we can't proceed with"
+ " the Jitted Methods analysis.");

Console.WriteLine("App Exit Code: {0}", appResult);
Console.WriteLine("Exiting...");

return 101;
}

// App finished successfully. We can now take a look at how many methods
// got jitted at runtime.
Console.WriteLine("Number of Jitted Methods: {0}\n", appResult);
return appResult > 0 && appResult <= MAX_JITTED_METHODS_ACCEPTED ? 100 : 101;
}

private static bool IsReadyToRunEnvSet()
{
string? dotnetR2R = Environment.GetEnvironmentVariable("DOTNET_ReadyToRun");
return (string.IsNullOrEmpty(dotnetR2R) || dotnetR2R == "1");
}

private static bool IsRunCrossgen2Set()
{
string? runCrossgen2 = Environment.GetEnvironmentVariable("RunCrossGen2");
return (runCrossgen2 == "1");
}

private static bool IsRunningOnARM64()
{
InteropServices.Architecture thisMachineArch = InteropServices
.RuntimeInformation
.OSArchitecture;

return (thisMachineArch == InteropServices.Architecture.Arm64);
}

private static int RunHelloWorldApp(string appName)
{
// The app's exit code is the number of jitted methods it got.
int appExitCode = -1;

// Set up our CoreRun call.
string coreRoot = Environment.GetEnvironmentVariable("CORE_ROOT");
string exeSuffix = OperatingSystem.IsWindows() ? ".exe" : "";
string coreRun = Path.Combine(coreRoot, $"corerun{exeSuffix}");

using (Process app = new Process())
{
var startInfo = new ProcessStartInfo
{
FileName = coreRun,
Arguments = appName,
CreateNoWindow = true,
UseShellExecute = false,
};

Console.WriteLine("\nLaunching Test App: {0} {1}", startInfo.FileName,
startInfo.Arguments);

app.StartInfo = startInfo;
app.Start();
app.WaitForExit();
appExitCode = app.ExitCode;
}

return appExitCode;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<DebugType>pdbonly</DebugType>
<Optimize>true</Optimize>
<JitOptimizationSensitive>true</JitOptimizationSensitive>
</PropertyGroup>

<ItemGroup>
<Compile Include="JittedMethodsCountingTest.cs" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="HelloWorld/HelloWorld.csproj">
<ReferenceOutputAssembly>false</ReferenceOutputAssembly>
<OutputItemType>Content</OutputItemType>
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</ProjectReference>
</ItemGroup>

</Project>