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

Keep response files used for crossgenning the framework #43183

Merged
merged 3 commits into from
Oct 10, 2020

Conversation

trylek
Copy link
Member

@trylek trylek commented Oct 8, 2020

Response files are a very useful artifact for reproing various
Crossgen / Crossgen2 compilation errors. I propose modifying the
test build scripts to retain these artifacts by using the --nocleanup
option and by skipping deletion of the entire output folder upon
the end of framework compilation. The Crossgen output folder is not
listed among the artifacts transferred to Helix so it should be a
non-concern w.r.t. Helix throughput. I have also put the cmd and sh
versions of the script in sync by adding "large-bubble" and "release"
flags to the Unix version.

Thanks

Tomas

/cc: @dotnet/crossgen-contrib

Response files are a very useful artifact for reproing various
Crossgen / Crossgen2 compilation errors. I propose modifying the
test build scripts to retain these artifacts by using the --nocleanup
option and by skipping deletion of the entire output folder upon
the end of framework compilation. The Crossgen output folder is not
listed among the artifacts transferred to Helix so it should be a
non-concern w.r.t. Helix throughput. I have also put the cmd and sh
versions of the script in sync by adding "large-bubble" and "release"
flags to the Unix version.

Thanks

Tomas
@@ -170,7 +170,7 @@ precompile_coreroot_fx()
__NumProc=$(nproc --all)
fi

local crossgenCmd="\"$__DotNetCli\" \"$CORE_ROOT/R2RTest/R2RTest.dll\" compile-framework -cr \"$CORE_ROOT\" --output-directory \"$CORE_ROOT/crossgen.out\" --target-arch $__BuildArch -dop $__NumProc"
local crossgenCmd="\"$__DotNetCli\" \"$CORE_ROOT/R2RTest/R2RTest.dll\" compile-framework -cr \"$CORE_ROOT\" --output-directory \"$CORE_ROOT/crossgen.out\" --large-bubble --release --target-arch $__BuildArch -dop $__NumProc"
Copy link
Member

Choose a reason for hiding this comment

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

I think the --nocleanup should be added here too, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, thanks for reminding me!

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

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

@trylek, everything in the crossgen.out directory is copied to the helix machines. See

https://helixde107v0xdeko0k025g8.blob.core.windows.net/helix-job-586ecf1a-97c7-4261-870e-9ed23c371721ea7b316d6c9426089/8c8bd01b-aa7b-424b-ac64-94799dda1546.zip

We probably should just avoid using the core_root directory for holding temporary state, and instead put the crossgen.out directory somewhere else.

@trylek
Copy link
Member Author

trylek commented Oct 8, 2020

Thanks David for the heads-up. I'm going to investigate why that's happening as I believe that our current scripts should filter the folder out according to the following clauses:

<RunTimeArtifactsIncludeFolders Include="/" />

@trylek
Copy link
Member Author

trylek commented Oct 9, 2020

OK, I see the problem, this scripts kicks in earlier, it just defines how to populate CORE_ROOT, not how to filter its contents when zipping it up for Helix. In such case I'll modify it as David suggested, by moving the intermediate crossgen outputs somewhere under artifacts\obj.

@davidwrighton
Copy link
Member

The change looks good, but I'm concerned by the fact that all the tests seem to be failing. Also, the Linux logs don't print out which assemblies were crossgened. You may want to make the outputs consistent between Unix and Windows.

@trylek
Copy link
Member Author

trylek commented Oct 9, 2020

I concur, I keep trying to figure out what the problem is; it's just somewhat tricky because so far none of my attempts at reproing the issue have been successful, and that includes manually reconstructing the Helix workitem environment based on the payloads and running it locally. I'll fix the logging as you suggest and I'll continue looking but I must admit that right now I'm quite desperate what else should I try.

@nattress
Copy link
Contributor

nattress commented Oct 9, 2020

I see in the helix log

+ dotnet /root/helix/work/correlation/xunit.console.dll JIT/CodeGenBringUpTests/JIT.CodeGenBringUpTests.XUnitWrapper.dll -parallel collections -nocolor -noshadow -xml testResults.xml -trait TestGroup=JIT.CodeGenBringUpTests
Fatal error. Verify_TypeLayout 'System.Object' failed to verify type layout
   at Xunit.ConsoleClient.Program.Main(System.String[])

It looks like we started running CG2 on the Xunit assemblies in Core_Root recently. That's a bit weird to do because we're likely Crossgening them against the framework under test in Core_root. On Helix we run Xunit against the shared framework, which is some other version of the framework libraries. I don't really get how System.Object changed enough to fail Verify_TypeLayout since it's a simple class but maybe we should try excluding Xunit libraries from our framework CG2 script.

@trylek trylek merged commit c6d887c into dotnet:master Oct 10, 2020
@trylek trylek deleted the R2RTestNoCleanup branch October 10, 2020 19:13
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
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.

4 participants