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.

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

Remove CLRThreadpoolHosted as it always returns false. #9899

Merged
merged 1 commit into from Mar 2, 2017

Conversation

@AustinWise
Copy link

AustinWise commented Mar 2, 2017

When commit 1476776, as part of #9603, removed all the code under FEATURE_INCLUDE_ALL_INTERFACES, it left behind several functions in src/vm/util.hpp that just return false. CLRThreadpoolHosted is one of them.

While the compiler is probably able to able to see that these methods always return false and remove dead code accordingly, some of these CLR*Hosted functions make parts of the coreclr more complicated than they need to be and harder for humans to understand. Would it be valuable to have all of these functions removed?

@jkotas
jkotas approved these changes Mar 2, 2017
Copy link
Member

jkotas left a comment

Thanks!

@janvorli janvorli merged commit 542ca8e into dotnet:master Mar 2, 2017
13 checks passed
13 checks passed
CentOS7.1 x64 Debug Build and Test Build finished.
Details
FreeBSD x64 Checked Build Build finished.
Details
Linux ARM Emulator Cross Debug Build Build finished.
Details
Linux ARM Emulator Cross Release Build Build finished.
Details
OSX x64 Checked Build and Test Build finished.
Details
Ubuntu x64 Checked Build and Test Build finished.
Details
Ubuntu x64 Formatting Build finished.
Details
Windows_NT arm Cross Debug Build Build finished.
Details
Windows_NT arm Cross Release Build Build finished.
Details
Windows_NT x64 Debug Build and Test Build finished.
Details
Windows_NT x64 Formatting Build finished.
Details
Windows_NT x64 Release Priority 1 Build and Test Build finished.
Details
Windows_NT x86 Checked Build and Test Build finished.
Details
@stephentoub
Copy link
Member

stephentoub commented Mar 2, 2017

Can we also get rid of:

inline static BOOL IsThreadPoolHosted()
{
return FALSE;
}

?

@AustinWise AustinWise deleted the AustinWise:removeCLRThreadpoolHosted branch Mar 2, 2017
@AustinWise
Copy link
Author

AustinWise commented Mar 2, 2017

@stephentoub I will try removing it in a follow-up PR.

YongseopKim added a commit to YongseopKim/coreclr that referenced this pull request Mar 10, 2017
jorive added a commit to guhuro/coreclr that referenced this pull request May 4, 2017
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.