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.

Sign up for GitHub

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

Jump to bottom

Turn on ReadyToRun #12361

Merged
merged 7 commits into from
Apr 22, 2020
Merged

Turn on ReadyToRun #12361

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e9acd1e
Turn on ReadyToRun
iSazonov Apr 17, 2020
85fe743
on crossgen non-PowerShell binaries
TravisEz13 Apr 17, 2020
948b5f8
Disable Start-ResGen
iSazonov Apr 20, 2020
c8e33d0
Revert "Disable Start-ResGen"
iSazonov Apr 20, 2020
fd7bac4
Disable Start-CrossGen
iSazonov Apr 20, 2020
9b264a9
Revert ResGen
iSazonov Apr 21, 2020
86a53e4
Make crossgen code work as expected
TravisEz13 Apr 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions PowerShell.Common.props
View file
Open in desktop
Expand Up @@ -97,6 +97,7 @@

<TargetFramework>netcoreapp5.0</TargetFramework>
<LangVersion>8.0</LangVersion>
<PublishReadyToRun>true</PublishReadyToRun>

<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
Expand Down
33 changes: 6 additions & 27 deletions build.psm1
View file
Open in desktop
Expand Up @@ -414,10 +414,11 @@ Fix steps:

# handle ResGen
# Heuristic to run ResGen on the fresh machine
if ($ResGen -or -not (Test-Path "$PSScriptRoot/src/Microsoft.PowerShell.ConsoleHost/gen")) {
Write-Log "Run ResGen (generating C# bindings for resx files)"
Start-ResGen
}
# TODO: remove Start-ResGen after testing ReadyToRun
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this change. This is the wrong PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, sorry.

#if ($ResGen -or -not (Test-Path "$PSScriptRoot/src/Microsoft.PowerShell.ConsoleHost/gen")) {
# Write-Log "Run ResGen (generating C# bindings for resx files)"
# Start-ResGen
#}

# Handle TypeGen
# .inc file name must be different for Windows and Linux to allow build on Windows and WSL.
Expand Down Expand Up @@ -2361,29 +2362,7 @@ function Start-CrossGen {
"Microsoft.ApplicationInsights.dll"
)

# Common PowerShell libraries to crossgen
$psCoreAssemblyList = @(
"pwsh.dll",
"Microsoft.PowerShell.Commands.Utility.dll",
"Microsoft.PowerShell.Commands.Management.dll",
"Microsoft.PowerShell.Security.dll",
"Microsoft.PowerShell.ConsoleHost.dll",
"System.Management.Automation.dll"
)

# Add Windows specific libraries
if ($environment.IsWindows) {
$psCoreAssemblyList += @(
"Microsoft.PowerShell.CoreCLR.Eventing.dll",
"Microsoft.WSMan.Management.dll",
"Microsoft.WSMan.Runtime.dll",
"Microsoft.PowerShell.Commands.Diagnostics.dll",
"Microsoft.PowerShell.GraphicalHost.dll",
"Microsoft.Management.Infrastructure.CimCmdlets.dll"
)
}

$fullAssemblyList = $commonAssembliesForAddType + $psCoreAssemblyList
$fullAssemblyList = $commonAssembliesForAddType
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems we should disable Start-CrossGen function.

Copy link
Member

Choose a reason for hiding this comment

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

We should probably disable Start-CrossGen until we can verify PublishReadyToRun is a good replacement

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

Choose a reason for hiding this comment

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

CrossGen and ReadyToRun are the same thing, well, it's a switch to crossgen.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can use set COMPlus_ReadyToRunLogFile=log.txt from issue I referenced above to check dll-s. Start-CrossGen does not create R2R dll-s.

Copy link
Member

Choose a reason for hiding this comment

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

@iSazonov crossgen with a switch is what is used to generate readytorun binaries in the dotnet project

Copy link
Member

Choose a reason for hiding this comment

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

And I switched us to that method, but per DotNet we need to use the csproj if we want our symbols to work: https://github.com/PowerShell/PowerShell/blob/master/build.psm1#L2217

Copy link
Member

Choose a reason for hiding this comment

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

But can your explain why crossgen should be disabled on the rest of the binaries?

Copy link
Member

Choose a reason for hiding this comment

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

I forgot that we also crossgen assemblies not part of the build that we include in the package. We should probably still crossgen those.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This make sense for third-party assemblies. For PSReadline and MI we could move to R2R format.

@iSazonov crossgen with a switch is what is used to generate readytorun binaries in the dotnet project

With set COMPlus_ReadyToRunLogFile=log.txt logs show that after Start-CrossGen our dll-s are not initialized as R2R but with PublishReadyToRun are initialized.


foreach ($assemblyName in $fullAssemblyList) {
$assemblyPath = Join-Path $PublishPath $assemblyName
Expand Down