Repository navigation
Converge the AssemblyLoadContext scenario - #1235
Conversation
Instead, rely on intercepting Assembly.Load
This test fails since we've disabled the JIT cache entirely.
This is temporary. We need to rewrite the test context to ensure the AssemblyLoadContext is used properly, since the default can no longer be initialized.
|
Fixed. |
|
Dongbo Wang (@daxian-dbw) I was hoping this would also fix #682, but it doesn't seem to. We'll need to investigate it. We should also document the generation of the type catalog, the assembly load context itself, and the alternatives we've investigated in the build internals document. |
There was a problem hiding this comment.
This is a super simple LINQ query. If we're worried about performance, wouldn't we want to generate it when generating the type catalog itself?
There was a problem hiding this comment.
Yes, it's about performance. 1. LINQ is slow, even though clean. I think we should avoid LINQ query as much as possible. 2. the LINQ query you used makes many unnecessary calls to "Substring", because there are a lot duplicates in 'coreClrTypeCatalog.Values'. String operation is expensive, and we can make it more efficient by getting the unique set first.
I would love to generate it as part of generating the type catalog. Doing so in Github is easy, but in windows build is pretty hard because it requires an FCIB to update the TypeCatalogGen.exe. I don't want to do it :)
There was a problem hiding this comment.
it requires an FCIB to update the TypeCatalogGen.exe
I barely know what that means and it sounds like pain.
String operation is expensive, and we can make it more efficient by getting the unique set first.
I'm sure we could do this in LINQ too :P it's just a list comprehension.
|
Andy Jordan (@andschwa) I assigned #682 to myself. This also repro in Nano PS. |
AppVeyor also does this, and it's the scenario our packages ship, so it's what we should test.
db625f2 to
fec2cb8
Compare
|
Travis is having OS X infrastructure problems. The Linux builds passed, I think we can consider this passing. sergei (@vors), Dongbo Wang (@daxian-dbw) shall we merge? |
|
Please note that it does disable xUnit tests until we've fixed how to start them 😦 |
|
|
Converge the AssemblyLoadContext scenario
This resolves #839.
Dongbo Wang (@daxian-dbw) and I aren't super happy with some of the changes we've had to make here, and are hoping we get better solutions from the .NET Core team soon, but this is our proposed solution for now.
Note that this all goes away when
GetAssembliescomes back, per #733.This is probably still a bit of a work-in-progress, Dongbo and I need to go back over it tomorrow.