Fixing the OOM in scripting

[jaredpar]

Dug into the Scripting OOM tests more and figured out what was going on. The scripting tests predominantly create PortableExecutableReference instances using Stream as the source. Under the hood this creates a lot of native allocations in the form of memory mapped files and AllocHGlobal memory. As PortableExecutableReference is not disposable, these allocations are cleaned up passively in finalizers.

This means that our tests rely on the finalizer running fast enough that it frees up enough native memory for the next scripting test that allocates native memory to succeed. When the scripting tests are run in isolation or in an x64 process this works out just fine. But when we run the tests on x86 and in combination with other test assemblies this can break down and lead to OOM situations.

This fix approaches the problem from three directions:

1. Scripting had an existing mechanism for intercepting files being converted to PortableExecutableReference. This change expands on that to make it more thoroughly plumbed through the scripting code base and to have a single place, ScriptOptions, where the intercept can be hooked up.
2. Creating a ScriptTestBase type which sets up the intercepting of file loads and then actively disposing the resulting MetadataImageReference on Dispose. This means the native memory is actively freed at the end of each test. It also provides a ScriptOptions value that derived tests can leverage.
3. Plumbing through ScriptOptions to the individual tests.

After doing this I ran the script tests under PerfView in a number of configurations and verified that the memory was being actively freed and the heap size of the process was reduced as expected.

[jaredpar]

@dotnet/roslyn-compiler PTAL

[333fred]

Can we use the BannedAPIAnalyzer to ban the non-options-taking versions of these methods in our test assemblies so we don't end up here again in 5 years?

[jaredpar]

Can we use the BannedAPIAnalyzer to ban the non-options-taking versions of these methods in our test assemblies so we don't end up here again in 5 years?

I don't think we can. I definitely considered this but the problem is that every overload has an options parameter, it's just that in some cases it's a default value. Don't think the banned API analyzer can deal with that.

[333fred]

Gotcha. This doesn't feel worth an analyzer to me, but we should document this somewhere; maybe a README in the scripting test project? Or a note in the base class?

[jaredpar]

@dotnet/roslyn-compiler PTAL
@333fred doc added.

[jjonescz]

Given that "MetadataImageReference over Stream results in a lot of native allocations as opposed to using an ImmutableArray<byte> which has none", have we considered fixing this by using ImmutableArray<byte> instead of relying on MetadataImageReference over Stream in the scripting engine?

[jaredpar]

I believe the reason scripting uses Stream is that in the normal use case the belief is that it should result in less overall allocations. The compiler lazily loads data from the PE file so in theory you could load a lot of assemblies with only targeted allocations. As opposed to the ImmutableArray<byte> case where you have to eagerly load them all into memory.

The unit tests are just taking the downsides of this approach to the extreme.

[jaredpar]

@dotnet/roslyn-ide can I get a review on the scripting side? This will resolve the OOM issues we've been seeing in CI

[dibarbet]

seems reasonable