Keep using RO metadata after creating IMetaDataImporter for PDB reading

[hoyosjs]

This change allows the runtime to keep using the read-only view of the metadata interfaces for internal use, even after a RW conversion of the metadata has been requested to back the classic PDB parser. Swapping of the cached RW converted view will be deferred until some other path (e.g. profiler, MD emit) requests a RW view of the metadata.

Main version of fix for #90563

[hoyosjs]

Following the repro in the original issue and deleting the PDB I observe on average 9k exceptions per second on my 12 core machine. After this change the rate grows to ~20k, which is in line with the perf of builds using portable PDBs with no PDB available.

[jkotas]

Can this be moved to the end of the method and merged with similar code that's there already?

I think the code would be much easier to understand if the method flow is like this:

// Step 1: Create RW MD import if it was not created yet
if (m_pConvertedMDImport == NULL)
{
    create and initialize m_pConvertedMDImport
}

// Step 2: Swap the RW import if the scope is being opened for writing and the RW import was not swapped yet
if (openForWriting && m_pConvertedMDImport != m_pMDImport)
{
...
}

[AaronRobinsonMSFT]

Does this API need a default value? Can we update all places with an argument?

[AaronRobinsonMSFT]

Let's assert m_pConvertedMDImport is non NULL and comment why we need the converted version.

[AaronRobinsonMSFT]

I still don't understand this part. I think we only get in here if m_pMDImport and m_pConvertedMDImport are non-null and aren't the same instance. If this is true, then both pConvertedImport and pOld were/are fields on this class. I would assume then that both have correct references counts for being on this class. I do see the pOld being passed into SetUserContextData() and this could be a lifetime transfer, but if that were the case, I would expect pOld to need an AddRef() and not pConvertedImport.

[AaronRobinsonMSFT]

The pNew was returned from above with an AddRef(), see GetMetaDataInternalInterfaceFromPublic(), why is this neded here? I think I get the case if pNew = m_pConvertedMDImport; is done, but in that case I would expect an pNew->AddRef() there as opposed to here.

[dotnet-policy-service]

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.