Allow creating a MetadataFile with a MetadataReader / MetadataStringDecoder - #3487
Merged
Conversation
| } | ||
|
|
||
| private protected MetadataFile(MetadataFileKind kind, string fileName, PEReader reader, MetadataReaderOptions metadataOptions = MetadataReaderOptions.Default) | ||
| public MetadataFile(MetadataFileKind kind, string fileName, MetadataReader metadataReader, MetadataReaderOptions metadataOptions = MetadataReaderOptions.Default, int metadataOffset = 0, bool isEmbedded = false) |
Member
There was a problem hiding this comment.
The metadataOptions parameter is unused in this overload.
Contributor
Author
There was a problem hiding this comment.
Thanks for catching this. I have pushed an additional commit that corrects this
Member
|
Overall, I think this is a good idea. I don't remember why I didn't add the |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
System.Reflection.Metadataallows specifying aSystem.Reflection.Metadata.MetadataStringDecoderto yourMetadataReaderto cache any strings it allocates, thereby reducing allocations.I have an existing
MetadataReaderthat is using my customMetadataStringDecoder, howeverICSharpCode.Decompiler.Metadata.MetadataFiledoes not allow me to pass in my existingMetadataReader, it instead requires that I pass in aSystem.Reflection.Metadata.MetadataReaderProvider.If
MetadataReaderProvider.GetMetadataReaderis called multiple times with the same inputs, the sameMetadataReaderwill be returned each time. I thought that maybe I could create aMetadataReaderProviderto get theMetadataReaderthat I andICSharpCode.Decompilerboth need to use, howeverGetMetadataReaderdoes not return its cachedMetadataReaderif it is called multiple times with different arguments.GetMetadataReaderallows specifying aMetadataStringDecoderto it, however,ICSharpCode.Decompilerdoes not support this, which meansMetadataReaderthat is shared byICSharpCode.Decompilerand code that runs outside of itICSharpCode.Decompilerwill create a lot of extra string allocations in my program that could be avoided with the use ofMetadataStringDecoderSolution
There are two potential ways this can be solved
MetadataStringDecoderparameter to existing code paths that lead to the creation of aMetadataFileMetadataFilethat allows specifying an existingMetadataReaderdirectlyThe former approach works well with the
PEReadertype, which derives fromMetadataFile, while the latter works better when you're handling PE parsing yourself and want to interact withMetadataFiledirectly. As such, I have implemented both approaches.utf8Decoderis the name thatSystem.Reflection.Metadatagives toMetadataStringDecoderparameters, so I have used the same name