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

Stop checking .ni.dll/exe on Core #6663

Merged
merged 2 commits into from
Jul 28, 2021
Merged

Conversation

rainersigwald
Copy link
Member

We believe these were remnants of old pre-crossgen file naming patterns that haven't been used in a very long time.

Removing the checks to avoid filesystem activity on files that almost certainly will not exist.

Fixes #5042.

We believe these were remnants of old pre-crossgen file naming patterns that haven't been used in a very long time.

Removing the checks to avoid filesystem activity on files that almost certainly will not exist.

Fixes dotnet#5042.
@rainersigwald rainersigwald added this to the 17.0 milestone Jul 12, 2021
@ladipro
Copy link
Member

ladipro commented Jul 12, 2021

I believe that crossgen still creates .ni.dll files. At least I remember getting them when I invoked the tool a few months back. @vitek-karas is it a valid scenario to crossgen an assembly, delete the .dll, and assembly-load the .ni.dll?

@rainersigwald
Copy link
Member Author

I guess the relevant question here is "is there a supported plugin scenario where we should be explicitly loading a .ni.dll, as opposed to letting the runtime handle it?

I don't see any .ni.dll files in the various .NET SDKs from 3.1.100 to 6.0.100-preview.4 I have handy locally.

@vitek-karas
Copy link
Member

I personally haven't seen .ni.dll in a while - but I'm no crossgen expert. @trylek should have a better idea is .ni.dll is still a thing. The runtime loader can load these just fine but as far as I know the current design is that:

  • Native images for a single assembly are always bundled into the assembly itself (result is one file, typically with just .dll extension)
  • Composite images (native image for multiple assemblies together) is a separate file (not sure about the extension actually) to which all the managed assemblies have a reference - so runtime will know what to look for.

@trylek
Copy link
Member

trylek commented Jul 19, 2021

For .ni.dll, Crossgen2 doesn't automatically force this naming scheme as early in Crossgen2 development @jkotas explicitly suggested we should drop the ".ni" naming scheme for end customers and only maintain it for internal testing. In addition to our various test build scripts that still expect the .ni bit it turns out that the DiaSymReader PDB processing library contains hard-coded bits checking for the .ni extension, @davidwrighton had to provide special hacks to make it work in Crossgen2 / R2RDump context. The biggest thing I'm aware of right now is the naming overlap between the IL PDB (these days probably a modern managed PDB right out of @tmat's factory) and the native (Crossgen2) legacy Windows-style PDB used to propagate method addresses to PerfView. I'm not yet aware of any known plan to overcome this dichotomy but I guess we should figure one out for .NET 7.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The native PDB problem is separate issue. It should not block this cleanup from being merged.

.NET Core can sometimes use .exe extensions but in that case
the file is not a managed assembly any more and thus can't
be loaded by our plugin loader.
@ladipro ladipro added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jul 27, 2021
@ladipro ladipro merged commit 9f91131 into dotnet:main Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove probing for .ni.dll when loading .NET Core assemblies
5 participants