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

Fix IBC data embedding#36450

Merged
MichalStrehovsky merged 1 commit into
dotnet:masterfrom
MichalStrehovsky:ngenopt
Mar 28, 2019
Merged

Fix IBC data embedding#36450
MichalStrehovsky merged 1 commit into
dotnet:masterfrom
MichalStrehovsky:ngenopt

Conversation

@MichalStrehovsky
Copy link
Copy Markdown
Member

An arcade change broke IBC data embedding and we were not doing it.

This fixes that issue and also bumps embedded IBC from being partial to full.

An arcade change broke IBC data embedding and we were not doing it.

This fixes that issue and also bumps embedded IBC from being `partial` to `full`.
<PropertyGroup Condition="'$(IsEligibleForNgenOptimization)' == ''">
<IsEligibleForNgenOptimization>true</IsEligibleForNgenOptimization>
<IsEligibleForNgenOptimization Condition="'$(IsReferenceAssembly)' == 'true'">false</IsEligibleForNgenOptimization>
<IsEligibleForNgenOptimization Condition="'$(GeneratePlatformNotSupportedAssembly)' == 'true' OR '$(GeneratePlatformNotSupportedAssemblyMessage)' != ''">false</IsEligibleForNgenOptimization>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something to consider here: today optimization data isn't represented by configuration, so we apply to same data to all builds: thus the need for these approximations of where not to apply the data. I think it would be a better solution if the configuration information was preserved, or at least we were more intentional about the cases where we wanted to apply the same optimization data to multiple assemblies.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Cc @brianrob on that.

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.

LGTM.

@MichalStrehovsky
Copy link
Copy Markdown
Member Author

Cc @sergiy-k @RussKeldorph

@MichalStrehovsky MichalStrehovsky merged commit 719eee9 into dotnet:master Mar 28, 2019
@MichalStrehovsky MichalStrehovsky deleted the ngenopt branch March 28, 2019 20:26
@karelz karelz added this to the 3.0 milestone Apr 1, 2019
MichalStrehovsky added a commit to MichalStrehovsky/corefx that referenced this pull request Apr 17, 2019
Ngen optimizations are controlled with two properties - one is EnableNgen, the other is ApplyNgen. We were already using the ApplyNgen property with value of `full` (as of dotnet#36450), so this was already doing full Ngen, but we still used EnablePartialNgen to control the EnableNgen property. It's a roundabout way to EnableNgen, but does the same thing. This pull request removes the indirection.
ericstj pushed a commit that referenced this pull request Apr 29, 2019
Ngen optimizations are controlled with two properties - one is EnableNgen, the other is ApplyNgen. We were already using the ApplyNgen property with value of `full` (as of #36450), so this was already doing full Ngen, but we still used EnablePartialNgen to control the EnableNgen property. It's a roundabout way to EnableNgen, but does the same thing. This pull request removes the indirection.
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
An arcade change broke IBC data embedding and we were not doing it.

This fixes that issue and also bumps embedded IBC from being `partial` to `full`.

Commit migrated from dotnet/corefx@719eee9
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…36962)

Ngen optimizations are controlled with two properties - one is EnableNgen, the other is ApplyNgen. We were already using the ApplyNgen property with value of `full` (as of dotnet/corefx#36450), so this was already doing full Ngen, but we still used EnablePartialNgen to control the EnableNgen property. It's a roundabout way to EnableNgen, but does the same thing. This pull request removes the indirection.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants