Enable running libraries tests with crossgen2 compiled runtime #34365

janvorli · 2020-04-01T09:22:06Z

This change adds support for precompiling testhost assemblies
for libraries tests with crossgen2. The msbuild property Crossgen2Host
is used to enable that (passing /p:Crossgen2Host=true to the build command)

This change adds support for precompiling testhost assemblies with crossgen2. The msbuild property Crossgen2Host is used to enable that (passing /p:Crossgen2Host=true to the build command)

ViktorHofer · 2020-04-01T09:48:20Z

src/libraries/pretest.proj

+    </ItemGroup>
+
+    <Move SourceFiles="@(FilesToCopy)" DestinationFolder="$(TestHostRuntimePath)" />
+    <RemoveDir Directories="$(TestHostRuntimePath)\crossgen2out" />


Can we use the same name that we used for System.Private.CoreLib which is/was native to store the crossgend assemblies. Also should we backup the IL version inside an IL folder and allow to get back to the IL state? Ie coverage measurement doesn't work with the crossgend assemblies and getting back to the non-crossgend state without rebuilding libraries would be desirable.

What about building crossgend without replacing the IL ones and allow to run on both modes with the switch? cc @ericstj

We actually do the same thing in coreclr build-test.cmd - we just overwrite the assemblies in Core_Root with their crossgened versions and don't save anything. I think that keeping them in a different folder would be quite difficult as I think it would require to make a copy of the whole testhost folder and then changing all the references to the testhost in our scripts to this new one.

We could possibly keep the IL versions, but it seems it would be tricky to manage that. At the beginning of test run without the crossgened assemblies, we would need to check if the IL folder was present and copy stuff from it back. That could result in stale files being copied - consider the following sequence:

I build libraries with crossgening the testhost. That would create the IL folder with copies of the assemblies.

I make some changes to coreclr or libraries and rebuild them without crossgening. Now the testhost contains updated non-crossgened assemblies.

I execute the tests - we find that there is an IL folder and copy the files back, overwriting the new ones with the stale ones. We cannot use time stamp of the files to check that, because we don't know if the files in the testhost are the crossgened ones or new IL ones.

I guess that we can find a way to make it work, but I doubt it is worth the hassle. I doubt that the flow when someone would need to run with and without crossgen without rebuilding libraries in between is important.

OK, so what's the overall plan here? Will we add scheduled runs that set the property to true to test with crossgend?

Right, that's what I would do once I fix the issues in crossgen2 that it has discovered.

Btw, since the crossgen2out is just a temporary folder that's deleted after the crossgen completes, would you still want to rename it to native? I don't really care about the name.

No if it's just temporary then it shouldn't matter. Thanks. If possible let's wait for ericstj's review.

Sure, no need to rush.

I think this is complex because you imagine modifying in place. It’s actually easier if you imagine crossgen producing a new shared framework with a different root (eg: testhost_crossgen vs testhost). Then it’s just a freshness check: is the non-CG newer? -> rerun crossgen. In general we try make the build consume immutable assets and produce new immutable assets. I’d prefer if we have a second folder.

trylek

To me this looks good.

janvorli · 2020-04-03T14:02:13Z

@ericstj can you please take a look? @ViktorHofer has asked me to wait for your review.

ericstj · 2020-04-03T14:16:15Z

src/libraries/pretest.proj

+    </ItemGroup>
+
+    <MakeDir Directories="$(TestHostRuntimePath)\crossgen2out" />
+    <Exec Command="$(TestHostRootPath)\dotnet $(TestHostRuntimePath)\crossgen2.dll %(FilesToCrossgen.FullPath) --targetarch=x64 -O --inputbubble -r:$(TestHostRuntimePath)\System.*.dll -r:$(TestHostRuntimePath)\Microsoft.*.dll -r:$(TestHostRuntimePath)\mscorlib.dll -r:$(TestHostRuntimePath)\netstandard.dll -o:$(TestHostRuntimePath)\crossgen2out\%(FilesToCrossgen.Filename)%(FilesToCrossgen.Extension)" 


This is running on the shared framework that it is modifying at the same time. Typically we do not use the built product to perform build operations. Should you use the stable build framework under .dotnet instead?

This command line probably needs quotes around paths. Also, can crossgen2 just accept a folder for -r? Seems fragile to require named references when it could just probe for them. Probing seems simpler than resolving the wildcard in the app which it seems to be doing now.

ericstj · 2020-04-03T14:18:26Z

src/libraries/restore/runtime/runtime.depproj

+      <CoreCLRFiles Include="$(CoreCLRArtifactsPath)/crossgen2/*mscordaccore_*.*" />
+      <CoreCLRFiles Include="$(CoreCLRArtifactsPath)/crossgen2/*jitinterface.*" />
+      <CoreCLRFiles Include="$(CoreCLRArtifactsPath)/crossgen2/*clrjitilc.*" />
+      <ReferenceCopyLocalPaths Include="@(CoreCLRFiles)" />


Why copy this? Does it need to be in the test shared framework? I would prefer to move in the direction of making our test framework closer to the real thing rather than dump more test specific changes to it.

ericstj · 2020-04-03T14:22:36Z

src/libraries/pretest.proj

+    </ItemGroup>
+
+    <Move SourceFiles="@(FilesToCopy)" DestinationFolder="$(TestHostRuntimePath)" />
+    <RemoveDir Directories="$(TestHostRuntimePath)\crossgen2out" />


I think this is complex because you imagine modifying in place. It’s actually easier if you imagine crossgen producing a new shared framework with a different root (eg: testhost_crossgen vs testhost). Then it’s just a freshness check: is the non-CG newer? -> rerun crossgen. In general we try make the build consume immutable assets and produce new immutable assets. I’d prefer if we have a second folder.

ericstj · 2020-04-03T14:23:55Z

src/libraries/pretest.proj

@@ -68,11 +68,31 @@
             ContinueOnError="ErrorAndContinue" />
  </Target>

+  <Target Name="RunCrossgen2OnHost" Condition="'$(Crossgen2Host)' == 'true'">


Why Pretest and not a step in each libraries project file? This makes things harder on inner loop if you care about crossgen2.

With #31712, we are going to be illink trimming all the assemblies in the shared framework together in a "post build" step.

We can't crossgen in a step in each libraries project file because that will run before we run the linker. We will need to run crossgen on the output of the linker.

davidwrighton · 2020-10-08T01:32:34Z

@janvorli Jan, are you planning to do work on this? If not, please consider closing the PR.

Enable running libraries tests with crossgen2 compiled runtime

51e0702

This change adds support for precompiling testhost assemblies with crossgen2. The msbuild property Crossgen2Host is used to enable that (passing /p:Crossgen2Host=true to the build command)

janvorli added the area-crossgen2-coreclr label Apr 1, 2020

janvorli added this to the 5.0 milestone Apr 1, 2020

janvorli requested review from ViktorHofer and trylek April 1, 2020 09:22

janvorli self-assigned this Apr 1, 2020

ViktorHofer reviewed Apr 1, 2020

View reviewed changes

ViktorHofer requested a review from ericstj April 1, 2020 11:28

trylek approved these changes Apr 1, 2020

View reviewed changes

ericstj reviewed Apr 3, 2020

View reviewed changes

eerhardt mentioned this pull request Apr 20, 2020

Run illink task with at the end of build #31712

Closed

janvorli marked this pull request as draft June 22, 2020 19:45

janvorli closed this Oct 12, 2020

ghost locked as resolved and limited conversation to collaborators Dec 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable running libraries tests with crossgen2 compiled runtime #34365

Enable running libraries tests with crossgen2 compiled runtime #34365

janvorli commented Apr 1, 2020

ViktorHofer Apr 1, 2020

janvorli Apr 1, 2020

ViktorHofer Apr 1, 2020

janvorli Apr 1, 2020

janvorli Apr 1, 2020

ViktorHofer Apr 1, 2020

janvorli Apr 1, 2020

ericstj Apr 3, 2020

trylek left a comment

janvorli commented Apr 3, 2020

ericstj Apr 3, 2020

ericstj Apr 3, 2020

ericstj Apr 3, 2020

ericstj Apr 3, 2020

eerhardt Apr 22, 2020

davidwrighton commented Oct 8, 2020

Enable running libraries tests with crossgen2 compiled runtime #34365

Enable running libraries tests with crossgen2 compiled runtime #34365

Conversation

janvorli commented Apr 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trylek left a comment

Choose a reason for hiding this comment

janvorli commented Apr 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidwrighton commented Oct 8, 2020