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

Conversation

Djuffin
Copy link

@Djuffin Djuffin commented Nov 5, 2015

DebuggerHeap is a common class that is used for both executable and
non-executable memory allocations by debugging infrustructure.
On Windows, OS supports concept of executable heap and CLR doesn't need
to do anything extra for each executable allocation. On Linux/OSX
this is not true. That's why this changes preserves fExecutable flag for each heap
and makes sure that we mark memory as executable only when it is necessary.

This change should address performance degradation described in #711.
It doesn't fix managed debugging on CentoOS, but allows to run CoreCLR processes there without constant SELinux warnings.

DebuggerHeap is a common class that is used for both executable and
non-executable memory allocations by debugging infrustructure.
On Windows, OS supports concept of executable heap and CLR doesn't need
to do anything extra for each executable allocation. On Linux/OSX
this is not true. That's why this changes preserves fExecutable flag for each heap
and makes sure that we mark memory as executable only when it is necessary.
@Djuffin
Copy link
Author

Djuffin commented Nov 5, 2015

This PR makes sure changes from #1963 get to RC1
/cc @tzwlai @mikem8361 @pgavlin

@mikem8361
Copy link

LGTM

@pgavlin
Copy link

pgavlin commented Nov 5, 2015

LGTM as well.

@ellismg
Copy link

ellismg commented Nov 6, 2015

@dotnet-bot test Ubuntu x64 Debug Build

ellismg added a commit that referenced this pull request Nov 6, 2015
Restrict allocation of executable memory by DebuggerHeap
@ellismg ellismg merged commit 8923762 into dotnet:release/1.0.0-rc1 Nov 6, 2015
@ellismg
Copy link

ellismg commented Nov 6, 2015

I merged this before the JIT tests completed. It seemed like they have been taking a long time.

@ellismg
Copy link

ellismg commented Nov 6, 2015

(They did pass on Windows Retail)

@ellismg
Copy link

ellismg commented Nov 6, 2015

Looks like there was a test failure @Djuffin, is this a real issue specific to the RC1 branch? Will we need to take another fix or roll this back?

@Djuffin
Copy link
Author

Djuffin commented Nov 6, 2015

@ellismg I'd be surprised to learn that my change made one JIT test on Windows timeout.
Especially given that actual behavior change was made only in code under FEATURE_PAL.

d:\j\workspace\dotnet_coreclr\debug_windows_nt_prtest\bin\tests\Windows_NT.x64.debug\JIT\CodeGenBringUpTests\LocallocLarge.cmd Timed Out Raw Output

@AndyAyersMS
Copy link
Member

@ellismg @Djuffin there is definitely something odd going on with the check build testing recently... see #1966.

I can't repro any slowdowns on the LocallocLarge test locally so suspect something about the CI configuration is off. Matt says he enabled some parallelism on windows so that could be it.

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.

6 participants