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

Garbage collect CoreLib resource strings #46121

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

MichalStrehovsky
Copy link
Member

Most seem to be orphaned after the deletion of Utf8String and built-in WinRT support.

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@MichalStrehovsky
Copy link
Member Author

Oh, seems like a lot of the garbage is Mono's garbage. Seems like after #2151 we embed garbage CoreCLR strings into Mono builds and garbage Mono strings into CoreCLR build.

The proper fix for that is still tracked in #7643. I'll bump that issue to .NET 6 and mark as linkable framework.

Most seem to be orphaned after the deletion of Utf8String and built-in WinRT support.
@MichalStrehovsky
Copy link
Member Author

I re-ran the script (the one that Stephen created/ran a couple years ago), including Mono this time too. I manually excluded resources that are dynamically computed here:

string? displayName = SR.GetResourceString("Globalization_cp_" + codePage.ToString());

@MichalStrehovsky
Copy link
Member Author

@stephentoub @danmosemsft could you have a look?

@MichalStrehovsky MichalStrehovsky merged commit c9a11fe into dotnet:master Dec 18, 2020
@danmoseley
Copy link
Member

@MichalStrehovsky could you stick the script here or a gist for it, for future use?

Ideally we would eliminate the places we dynamically construct resource ID's in our tree, then we could trivially run this periodically on the tree. It was not hard to find other dead strings earlier this week, eg. #46186

@MichalStrehovsky
Copy link
Member Author

@MichalStrehovsky could you stick the script here or a gist for it, for future use?

It was this one: dotnet/coreclr#18808 (comment)

I just copied Mono's CoreLib into CoreCLR's directory and I have the symbolic links described here as well:

runtime/.gitignore

Lines 343 to 351 in fcf0b8b

# Symbolic link for the shared portion of CoreLib to make grep/findstr work for runtime devs
#
# On Windows, make your own by running these commands from the repo root:
# mklink /D src\coreclr\System.Private.CoreLib\shared %CD%\src\libraries\System.Private.CoreLib\src
# mklink /D src\coreclr\System.Private.CoreLib\common %CD%\src\libraries\Common\src
#
# On Unix, make your own by running these commands from the repo root:
# ln -s $(pwd)/src/libraries/System.Private.CoreLib/src src/coreclr/System.Private.CoreLib/shared
# ln -s $(pwd)/src/libraries/Common/src src/coreclr/System.Private.CoreLib/common

@ghost ghost locked as resolved and limited conversation to collaborators Jan 17, 2021
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.

4 participants