-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[NO-MERGE / DESIGN DISCUSSION ONLY] Infra fixes for Ivan's JIT count test #90647
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
Changes from all commits
c7c02a5
c7df757
9b9bb10
1713f45
fd476f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| // 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!"); | ||
| return 0; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <CLRTestKind>BuildOnly</CLRTestKind> | ||
| <TargetFramework>$(NetCoreAppToolCurrent)</TargetFramework> | ||
| <!-- <GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles> --> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Compile Include="HelloWorld.cs" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // 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 System.Reflection; | ||
| using Xunit; | ||
|
|
||
| public class JittedMethodsCountingTest | ||
| { | ||
| private const int MAX_JITTED_METHODS_ACCEPTED = 50; | ||
|
|
||
| [Fact] | ||
| public static int TestEntryPoint() | ||
| { | ||
| string testAppFolder = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); | ||
|
|
||
| string appName = Path.Combine(testAppFolder, "HelloWorld.dll"); | ||
| string jitOutputFile = Path.Combine(testAppFolder, "jits.txt"); | ||
|
|
||
| int appResult = RunHelloWorldApp(appName, jitOutputFile); | ||
| int jits = GetNumberOfJittedMethods(jitOutputFile); | ||
|
|
||
| return jits > 0 && jits <= MAX_JITTED_METHODS_ACCEPTED ? 100 : 101; | ||
| } | ||
|
|
||
| private static int RunHelloWorldApp(string appName, string jitOutputFile) | ||
| { | ||
| int appExitCode = -1; | ||
| string osExeSuffix = (OperatingSystem.IsWindows() ? ".exe" : ""); | ||
| string coreRootPath = Environment.GetEnvironmentVariable("CORE_ROOT"); | ||
| string coreRunPath = Path.Combine(coreRootPath, "corerun" + osExeSuffix); | ||
|
|
||
| using (Process app = new Process()) | ||
| { | ||
| var startInfo = new ProcessStartInfo | ||
| { | ||
| FileName = coreRunPath, | ||
| Arguments = appName, | ||
| CreateNoWindow = true, | ||
| UseShellExecute = false, | ||
| }; | ||
|
|
||
| Console.WriteLine("Launching: {0} {1}", startInfo.FileName, startInfo.Arguments); | ||
|
|
||
| startInfo.EnvironmentVariables.Add("DOTNET_JitDisasmSummary", "1"); | ||
| startInfo.EnvironmentVariables.Add("DOTNET_TieredCompilation", "0"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be testing the default config (ie |
||
| startInfo.EnvironmentVariables.Add("DOTNET_ReadyToRun", "1"); | ||
| startInfo.EnvironmentVariables.Add("DOTNET_JitStdOutFile", jitOutputFile); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried this without the stdout file redirection and it seemed to work for me when I specified
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think using
|
||
|
|
||
| app.StartInfo = startInfo; | ||
| app.Start(); | ||
| app.WaitForExit(); | ||
|
|
||
| appExitCode = app.ExitCode; | ||
| Console.WriteLine("Exit code: {0}", appExitCode); | ||
| Console.WriteLine("JIT file: {0} ({1} B)", jitOutputFile, new FileInfo(jitOutputFile).Length); | ||
| } | ||
|
|
||
| return appExitCode; | ||
| } | ||
|
|
||
| private static int GetNumberOfJittedMethods(string jitOutputFile) | ||
| { | ||
| string[] lines = File.ReadAllLines(jitOutputFile); | ||
|
|
||
| Console.WriteLine("=========== App standard output ==========="); | ||
| foreach (string line in lines) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I leave it up to you how verbose you decide the output should be but please keep in mind that sufficient diagnostic information can typically substantially speed up bug investigation.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was planning to print everything afterwards. I only had the short output you saw first to make it easier to see whether the test was running appropriately. |
||
| { | ||
| Console.WriteLine(line); | ||
| } | ||
| Console.WriteLine("=========== End of app standard output ==========="); | ||
|
|
||
| string[] tokens = lines.Last().Split(":"); | ||
|
|
||
| int numJittedMethods = Int32.Parse(tokens[0]); | ||
| return numJittedMethods; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <DebugType>pdbonly</DebugType> | ||
| <Optimize>true</Optimize> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Compile Include="JittedMethodsCountingTest.cs" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="HelloWorld/HelloWorld.csproj"> | ||
| <ReferenceOutputAssembly>false</ReferenceOutputAssembly> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This clause is what I believe should tell the merged wrapper generator that we should skip this reference as it's not supposed to be a direct project component.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might also need to change the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, is that somehow related to the next line or something completely different?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, didn't see that under the comment. You're right, nevermind. |
||
| <OutputItemType>Content</OutputItemType> | ||
| <CopyToOutputDirectory>Always</CopyToOutputDirectory> | ||
| </ProjectReference> | ||
| </ItemGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <ProjectAssetsFile>$(JitPackagesConfigFileDirectory)benchmark/obj/project.assets.json</ProjectAssetsFile> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know if this is actually necessary? I don't think we're using it in other tests. |
||
| </PropertyGroup> | ||
| </Project> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // TODO: Add the Microsoft License header here. | ||
|
|
||
| using System; | ||
| using System.Diagnostics; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using Xunit; | ||
|
|
||
| public class JittedMethodsCounter | ||
| { | ||
| [Fact] | ||
| public static int TestEntryPoint() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about this one, is that an earlier version of the test you're working on now so that we should ultimately get rid of it or are you envisioning two different modes of testing?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going to be removed. It slipped through. The actual test is the other file you reviewed. |
||
| { | ||
| string appName = "HelloWorld"; | ||
| string jitOutputFile = "jits.txt"; | ||
| int testReturnCode = 0; | ||
|
|
||
| int appResult = RunHelloWorldApp(appName, jitOutputFile); | ||
| int jits = GetNumberOfJittedMethods(jitOutputFile); | ||
|
|
||
| Console.WriteLine($"\nApp's Exit Code: {appResult}"); | ||
| Console.WriteLine($"App's Total Jitted Methods: {jits}"); | ||
|
|
||
| if (jits < 100) | ||
| { | ||
| testReturnCode = 100; | ||
| Console.WriteLine("Less than 100 methods were Jitted so the test" | ||
| + " has passed!"); | ||
| } | ||
| else | ||
| { | ||
| testReturnCode = 101; | ||
| Console.WriteLine("More than 100 methods were Jitted so the test" | ||
| + " does not pass."); | ||
| } | ||
|
|
||
| return testReturnCode; | ||
| } | ||
|
|
||
| private static int RunHelloWorldApp(string appName, string jitOutputFile) | ||
| { | ||
| int exitCode = -1; | ||
|
|
||
| using (Process app = new Process()) | ||
| { | ||
| var startInfo = new ProcessStartInfo | ||
| { | ||
| FileName = appName, | ||
| CreateNoWindow = true, | ||
| UseShellExecute = false, | ||
| }; | ||
|
|
||
| startInfo.EnvironmentVariables.Add("DOTNET_JitDisasmSummary", "1"); | ||
| startInfo.EnvironmentVariables.Add("DOTNET_TieredCompilation", "0"); | ||
| startInfo.EnvironmentVariables.Add("DOTNET_ReadyToRun", "1"); | ||
| startInfo.EnvironmentVariables.Add("DOTNET_JitStdOutFile", jitOutputFile); | ||
|
|
||
| app.StartInfo = startInfo; | ||
| app.Start(); | ||
| app.WaitForExit(); | ||
|
|
||
| exitCode = app.ExitCode; | ||
| } | ||
|
|
||
| return exitCode; | ||
| } | ||
|
|
||
| private static int GetNumberOfJittedMethods(string jitOutputFile) | ||
| { | ||
| string[] tokens = File.ReadLines(jitOutputFile) | ||
| .Last() | ||
| .Split(":"); | ||
|
|
||
| int numJittedMethods = Int32.Parse(tokens[0]); | ||
| return numJittedMethods; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <DebugType>pdbonly</DebugType> | ||
| <Optimize>true</Optimize> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <Compile Include="$(MSBuildProjectName).cs" /> | ||
| </ItemGroup> | ||
| <PropertyGroup> | ||
| <ProjectAssetsFile>$(JitPackagesConfigFileDirectory)benchmark\obj\project.assets.json</ProjectAssetsFile> | ||
| </PropertyGroup> | ||
|
|
||
| </Project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // TODO: Add the Microsoft License header here. | ||
|
|
||
| using System; | ||
| using System.Diagnostics; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using Xunit; | ||
|
|
||
| public class JittedMethodsCountingTest | ||
| { | ||
| [Fact] | ||
| public static int TestEntryPoint() | ||
| { | ||
| int testReturnCode = 0; | ||
| return testReturnCode; | ||
| } | ||
|
|
||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <DebugType>pdbonly</DebugType> | ||
| <Optimize>true</Optimize> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Compile Include="$(MSBuildProjectName).cs" /> | ||
| </ItemGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <ProjectAssetsFile>$(JitPackagesConfigFileDirectory)benchmark/obj/project.assets.json</ProjectAssetsFile> | ||
| </PropertyGroup> | ||
| </Project> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that I added a check that the number of JITted methods is nonzero. I firmly believe it's even beyond the scope of .NET 9 to achieve JIT runtime count being zero and, while I'll be the first person to celebrate the milestone, for now the zero value most likely means something has just malfunctioned. Please take it as a reviewer suggestion to consider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know 0 meant an error in this case. Thanks for the observations Tomas!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I'm not saying that zero is strictly wrong, a time may come when, say, "dotnet new webapi" will indeed runtime JIT 0 methods at startup (meaning that Crossgen2 precompilation has become "perfect" for this particular app), however that is not happening today and my feeling is that hitting zero today would much more likely signify an infra issue like the log not being generated or properly parsed. I'm looking forward to the time when 0 is the expected value and anything else is investigated as a Crossgen2 bug :-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be part of the celebration. That day I adjust this test so that zero jitted methods can be a potentially expected result :)