Fix Windows coreclr.dll size regression #40282
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A recent change has replaced usage of SELECTANY by constexpr to significantly reduce size of coreclr binaries on Unix. But it has resulted in about 180kB regression in coreclr.dll size on Windows. This change gets rid of the constexpr at places that can be reasonably moved to having declaration in header and definition in a .cpp file. I've also found a couple of things that are not used anymore, so I have deleted them. The only thing where I've moved back to using SELECTANY on Windows is the DbgIPCEventTypeNames in dbgipcevents.h. That table and the related GetEventType / GetEventName methods are used in both mscordbi and coreclr and sharing a .cpp source with the data between those two libraries was ugly and problematic.
Member
Author
|
cc: @dotnet/jit-contrib for the jit changes. |
jkotas
reviewed
Aug 3, 2020
| // 32-bit magic numbers, (because the algorithm for using 33-bit magic numbers is slightly slower). | ||
| // | ||
|
|
||
| PrimeInfo primeInfo[] = |
jkotas
reviewed
Aug 3, 2020
| }; | ||
|
|
||
| constexpr IPCEventTypeNameMapping DbgIPCEventTypeNames[] = | ||
| #ifdef TARGET_WINDOWS |
Member
There was a problem hiding this comment.
Nit: #if defined(_MSC_VER) may be more appropriate
defined(_MSC_VER)
Jacksondr5
pushed a commit
to Jacksondr5/runtime
that referenced
this pull request
Aug 10, 2020
* Fix Windows coreclr.dll size regression A recent change has replaced usage of SELECTANY by constexpr to significantly reduce size of coreclr binaries on Unix. But it has resulted in about 180kB regression in coreclr.dll size on Windows. This change gets rid of the constexpr at places that can be reasonably moved to having declaration in header and definition in a .cpp file. I've also found a couple of things that are not used anymore, so I have deleted them. The only thing where I've moved back to using SELECTANY on Windows is the DbgIPCEventTypeNames in dbgipcevents.h. That table and the related GetEventType / GetEventName methods are used in both mscordbi and coreclr and sharing a .cpp source with the data between those two libraries was ugly and problematic. * Fix x86 build * Fix clang format * Reflect PR feedback defined(_MSC_VER)
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A recent change has replaced usage of SELECTANY by constexpr to
significantly reduce size of coreclr binaries on Unix. But it has
resulted in about 180kB regression in coreclr.dll size on Windows.
This change gets rid of the constexpr at places that can be reasonably
moved to having declaration in header and definition in a .cpp file.
I've also found a couple of things that are not used anymore, so I have
deleted them.
The only thing where I've moved back to using SELECTANY on Windows is
the DbgIPCEventTypeNames in dbgipcevents.h. That table and the related
GetEventType / GetEventName methods are used in both mscordbi and
coreclr and sharing a .cpp source with the data between those two
libraries was ugly and problematic.
Close #39599