Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Run IBCMerge on Linux build#36763

Merged
MichalStrehovsky merged 2 commits into
dotnet:masterfrom
MichalStrehovsky:ibcMerge
Apr 11, 2019
Merged

Run IBCMerge on Linux build#36763
MichalStrehovsky merged 2 commits into
dotnet:masterfrom
MichalStrehovsky:ibcMerge

Conversation

@MichalStrehovsky
Copy link
Copy Markdown
Member

@MichalStrehovsky MichalStrehovsky commented Apr 10, 2019

  • Restores IBC data for both Linux and Windows on all platforms
  • Chooses the right IBC data based on the target OS
  • When target OS isn't anything specific (like, e.g. System.Linq and other assemblies that are not OS specific), embeds Windows IBC data

@MichalStrehovsky
Run IBCMerge on Linux build
f167a87 
* Restores IBC data for both Linux and Windows on all platforms
* Chooses the right IBC data based on the target OS
* When target OS isn't anything specific (like, e.g. System.Linq and other assemblies that are not OS specific), embeds Windows IBC data
@MichalStrehovsky
Copy link
Copy Markdown
Member Author

MichalStrehovsky commented Apr 10, 2019

Cc @sergiy-k @RussKeldorph @brianrob

ericstj
ericstj approved these changes Apr 10, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Just double check your condition.

Comment thread eng/codeOptimization.targets Outdated
@Wraith2
Copy link
Copy Markdown
Contributor

Wraith2 commented Apr 10, 2019

What is IBC data?

@filipnavara
Copy link
Copy Markdown
Member

filipnavara commented Apr 11, 2019

What is IBC data?

Citing @MichalStrehovsky from Gitter: "IBC = Instrumented Block Counts. Basically, profile guided optimizations for managed code."

@MichalStrehovsky MichalStrehovsky merged commit ef2ee12 into dotnet:master Apr 11, 2019
@MichalStrehovsky MichalStrehovsky deleted the ibcMerge branch April 11, 2019 12:08
@ViktorHofer
Copy link
Copy Markdown
Member

ViktorHofer commented Apr 11, 2019

This broke the official builds: https://dnceng.visualstudio.com/internal/_build/results?buildId=151325

@MichalStrehovsky
Copy link
Copy Markdown
Member Author

MichalStrehovsky commented Apr 11, 2019

Well that's interesting.

/__w/1/s/.packages/microsoft.dotnet.arcade.sdk/1.0.0-beta.19207.1/tools/OptimizationData.targets(176,5): error MSB3073: The command ""/__w/1/s/.dotnet/dotnet" exec "/__w/1/s/.packages/microsoft.dotnet.ibcmerge/5.0.6-beta.19203.1/tools/netcoreapp2.0/ibcmerge.dll" -q -f  -minify -delete -mo "/__w/1/s/artifacts/tmp/netcoreapp-Release/OptimizedAssemblies/f5846cd9-09cb-4bff-80f4-5a9cc2e9169e" "/__w/1/s/artifacts/ibc/optimization.windows_nt-x64.IBC.CoreFx/data/System.Collections.NonGeneric.dll/System.Collections.NonGeneric.ibc"" exited with code 129. [/__w/1/s/src/System.Collections.NonGeneric/src/System.Collections.NonGeneric.csproj]
  The application to execute does not exist: '/__w/1/s/.packages/microsoft.dotnet.ibcmerge/5.0.6-beta.19203.1/tools/netcoreapp2.0/ibcmerge.dll'

The way this is supposed to work (and I actually tested this on both Windows and Linux in both configs) is that there are two options:

  1. Ibcmerge.dll was not restored (because we didn't restore the internal tools). We don't run it and just print a message with the arguments we would use to invoke it.
  2. Ibcmerge.dll exists. We run it.

Obviously, there's a secret option 3: Ibcmerge was restored on the official build machine where ibcmerge.dll has a quantum state. This does not spark joy.

MichalStrehovsky added a commit to MichalStrehovsky/corefx that referenced this pull request Apr 11, 2019
@MichalStrehovsky
Revert "Run IBCMerge on Linux build (dotnet#36763)"
571b49d 
This reverts commit ef2ee12.
@MichalStrehovsky
Copy link
Copy Markdown
Member Author

MichalStrehovsky commented Apr 11, 2019

Rollback in #36791.

stephentoub pushed a commit that referenced this pull request Apr 11, 2019
@MichalStrehovsky @stephentoub
Revert "Run IBCMerge on Linux build (#36763)" (#36791)
8e84331 
This reverts commit ef2ee12.
@ericstj
Copy link
Copy Markdown
Member

ericstj commented Apr 11, 2019

/cc @safern looks like this is a problem with the internal tools restore on linux.

@ericstj
Copy link
Copy Markdown
Member

ericstj commented Apr 11, 2019

Just had a look at the different steps and there's definitely something wrong here:

https://dev.azure.com/dnceng/internal/_build/results?buildId=151325&view=logs&jobId=7cc86366-6ece-5781-60ca-3d1c814b1449&taskId=7388ea57-952b-5299-49bb-4b8b88a97717&lineStart=8&lineEnd=16&colStart=1&colEnd=48

https://dev.azure.com/dnceng/internal/_build/results?buildId=151325&view=logs&jobId=338934ee-6440-5778-bafb-9371fbcccfa1&taskId=a2a8a210-de79-5606-58f6-9ddff1384723&lineStart=15&lineEnd=15&colStart=1&colEnd=260

So it looks like the Linux legs don't even attempt to run nuget at all.

@ViktorHofer
Copy link
Copy Markdown
Member

ViktorHofer commented Apr 11, 2019

Yes internal tools restore currently only works on Windows as we use nuget.exe in the yml step. Locally we use a crossplat solution. I will check what we can do here.

@safern
Copy link
Copy Markdown
Member

safern commented Apr 11, 2019

use a crossplat solution. I will check what we can do here.

I'm already in it... @chcosta is helping me set some service connections or an upstream in dnceng that uses the feed. Instead of using nuget.exe, I have changed it locally to use the .net core task to restore.

@safern safern mentioned this pull request Apr 13, 2019
Merged
@sergiy-k
Copy link
Copy Markdown

sergiy-k commented Apr 16, 2019

Given that #36840 has been merged, can we try enabling IBCMerge on Linux again?

@ViktorHofer
Copy link
Copy Markdown
Member

ViktorHofer commented Apr 16, 2019

Yes 👍

MichalStrehovsky added a commit to MichalStrehovsky/corefx that referenced this pull request Apr 16, 2019
@MichalStrehovsky
Revert "Revert "Run IBCMerge on Linux build (dotnet#36763)" (dotnet#3…
7b3f916 
…6791)"

This reverts commit 8e84331.
@MichalStrehovsky
Copy link
Copy Markdown
Member Author

MichalStrehovsky commented Apr 16, 2019

#36924 has the revert of the revert.

@karelz karelz added this to the 3.0 milestone May 4, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@MichalStrehovsky
Run IBCMerge on Linux build (dotnet/corefx#36763)
092ef15 
* Restores IBC data for both Linux and Windows on all platforms
* Chooses the right IBC data based on the target OS
* When target OS isn't anything specific (like, e.g. System.Linq and other assemblies that are not OS specific), embeds Windows IBC data

Commit migrated from dotnet/corefx@ef2ee12
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@MichalStrehovsky @stephentoub
Revert "Run IBCMerge on Linux build (dotnet/corefx#36763)" (dotnet/co…
8ac9612 
…refx#36791)

This reverts commit dotnet/corefx@ef2ee12.

Commit migrated from dotnet/corefx@8e84331
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@ericstj ericstj ericstj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

8 participants

@MichalStrehovsky @Wraith2 @filipnavara @ViktorHofer @ericstj @safern @sergiy-k @karelz