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

Fix for #12609 - add option to disable tiered compilation for profilers #14643

Merged
merged 4 commits into from
Oct 23, 2017
Merged

Fix for #12609 - add option to disable tiered compilation for profilers #14643

merged 4 commits into from
Oct 23, 2017

Conversation

davmason
Copy link
Member

@davmason davmason commented Oct 20, 2017

Fix for #12609

@@ -514,6 +514,7 @@ enum __MIDL___MIDL_itf_corprof_0000_0000_0006
COR_PRF_HIGH_ADD_ASSEMBLY_REFERENCES = 0x1,
COR_PRF_HIGH_IN_MEMORY_SYMBOLS_UPDATED = 0x2,
COR_PRF_HIGH_MONITOR_DYNAMIC_FUNCTION_UNLOADS = 0x4,
COR_PRF_HIGH_DISABLE_TIERED_COMPILATION = 0x8,
COR_PRF_HIGH_REQUIRE_PROFILE_IMAGE = 0,
COR_PRF_HIGH_ALLOWABLE_AFTER_ATTACH = ( COR_PRF_HIGH_IN_MEMORY_SYMBOLS_UPDATED | COR_PRF_HIGH_MONITOR_DYNAMIC_FUNCTION_UNLOADS ) ,
COR_PRF_HIGH_MONITOR_IMMUTABLE = 0
Copy link
Member

Choose a reason for hiding this comment

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

The new flag should be immutable because a method's eligibility for tiered compilation is also immutable. It doesn't look like COR_PRF_HIGH_MONITOR_IMMUTABLE is correctly hooked up, but you can copy what COR_PRF_MONITOR_IMMUTABLE does. The basic premise is that the flag can only be set at startup, and if you do then the profiler has permanently altered the default runtime configuration and isn't allowed to detach.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated to mimic what COR_PRF_MONITOR_IMMUTABLE does. I can't test the profiler detach path since we don't have profiler attach, but I at least verified that if you call SetEventMask2 and try to change the state it will return E_FAIL and abort

@noahfalk
Copy link
Member

LGTM

@davmason davmason merged commit 67a6615 into dotnet:master Oct 23, 2017
@davmason davmason deleted the disable_tiered branch October 24, 2017 03:24
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.

3 participants