Remove DynamicDependency from ValueType in browser builds

[marek-safar]

By removing global [DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.JSFunctionBinding", "System.Runtime.InteropServices.JavaScript")] attribute from System.ValueType we could keep only marshaling support for types that are used. The attribute needs to be used today because there is no reference to the assembly from managed code.

[dotnet-issue-labeler[bot]]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to 'arch-wasm': @lewing See info in area-owners.md if you want to be subscribed.

Issue Details

---

Author:	marek-safar
Assignees:	-
Labels:	`arch-wasm`, `area-CoreLib-mono`, `untriaged`
Milestone:	-

[ghost]

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar, @tannergooding See info in area-owners.md if you want to be subscribed.

Issue Details

---

By removing global `[DynamicDependency(DynamicallyAccessedMemberTypes.All, "System.Runtime.InteropServices.JavaScript.Runtime", "System.Private.Runtime.InteropServices.JavaScript")] ` attribute from System.ValueType we could keep only marshaling support for types that are used. The attribute needs to be used today because there is no reference to the assembly from managed code.

Author:	marek-safar
Assignees:	kg
Labels:	`arch-wasm`, `area-CoreLib-mono`, `size-reduction`
Milestone:	-

[pavelsavara]

The landscape changed and the ValueType now points to public type JSFunctionBinding which points to internal JavaScriptExports. The situation is bit better because the JavaScriptExports contains only methods which need to be reachable from JavaScript.

In scenarios which use interop code directly, like projects with Console, HTTP client or WebSocket client naturally use JSFunctionBinding from the generated [JSImport] code.

But we still have the legacy interop code, which depends on many things in the JavaScriptExports class. So at least for now, I don't think we could get rid of DynamicDependency on ValueType.

[pavelsavara]

We discussed this with @vitek-karas.

We could add [ModuleInitializer] dummy method into System.Runtime.InteropServices.JavaScript assembly, which would use DynamicDependency to protect JSFunctionBinding and JavaScriptExports classes.

Second option (not prefered) is to add linker root into XML like here

[vitek-karas]

Just to clarify the proposed approaches:

• Ideally there should be a direct code reference to the type in question (JSFunctionBinding for example). After all, something must have a need for it (otherwise we would not need to keep it).
• If that "need" is expressed via a reflection call, consider if it's possible to change it to use direct reference and avoid reflection. Not only it will solve all of the trimming problems, but it's faster (reflection is typically slow)
• If that's not possible and we need a reflection call (for example to break cyclic dependency), consider adding the DynamicDependency attribute onto the code which uses the reflection - this will allow the trimmer to remove the code and its references when it's not used by the app.
• If that is not possible - well that should typically only happen if a non-managed code needs to express a reference to managed code (native, JS, ...). In that case some kind of explicit rooting can be used. Ideally such solution should be done in code - can be done via module initializer for example. Or the potentially new assembly::DynamicDependency (which doesn't exist yet). In such case consider when will the trimmer be allowed to remove the "rooted" code - we should avoid code which is always rooted if at all possible. In these cases (external code reference) we typically use feature switches to make the dependency "conditional".
• Only as last resort the XML descriptor should be used - the reason we don't want to use this is that it's not visible from the code and thus is harder to maintain (note that XML descriptor can still be used in conjunction with feature switches to avoid always rooted code)

[pavelsavara]

Unfortunately [ModuleInitializer] doesn't work for this case, because there is no root which would protect the whole assembly from trimming. I guess the assembly::DynamicDependency would have the same issue.

[pavelsavara]

And the XML descriptor doesn't help either for the same reason. :(

[pavelsavara]

We could add conditional XML descriptor to corlib, but it's not much better than the current state. I guess the goal here is to encapsulate the solution in the System.Runtime.InteropServices.JavaScript assembly. I don't know how to do that.

[marek-safar]

Could we add the dependency to source generated code?

[vitek-karas]

The only thing I can think of is for the source generator to add something like AssemblyMetadata("IsTrimmable", "false"), but that defeats the purpose as that would keep the entire assembly, would not trim anything from it.

It's also questionable if the goal should be to root all assemblies which have JSExport in them - what if I use library which has that, but I don't need that part... I would have no way to get rid of such assembly.

ASP.NET has something called application parts (https://learn.microsoft.com/en-us/aspnet/core/mvc/advanced/app-parts?view=aspnetcore-6.0) which is basically a way to register components with the app. I vaguely remember that there some build-time component associated with these so that the components are made part of the app. In this case there's no need to register the assemblies (as they're going to be refereced as packages), but there is still a need to tell the system somehow to enable JS interop on them.

@sbomer for additional ideas...

[sbomer]

My understanding is that there's a legacy scenario where JavaScriptExports is only called from JS, and that there's nothing in managed code that indicates this may happen. If that's correct, then I think a conditional XML descriptor in corlib is currently the best way to express this.

It sounds like [JSExport] is the non-legacy scenario where there are generated JS wrappers but still no managed callers. For that scenario (assuming I understood more or less correctly), would it make sense for the generator to add DynamicDependency from the user code module initializer to each exported method?

[vitek-karas]

The problem is if this happens across assemblies. For example let's say I have assembly MyJSHelper which has some JSExport methods. Then in my app I include MyJSHelper as a package reference, but not direct reference to anything in the assembly, instead my app has some JS code which calls some of the JSExport methods from MyJSHelper.

There's nothing in managed world which would express the dependency between the app and MyJSHelper. And source generators won't help, because when it runs on MyJSHelper it sees the JSExport but can't "root" the assembly itself (you're just building a library). And when you run it on the app - there's no JSExport anywhere, so nothing the generator can react to.

[sbomer]

I see, then I think the linker fundamentally doesn't have insight into which JSExports may be called. The only automated way I can think of to solve this is for a JS bundler to determine which JS entry points are used, then for a component of our tooling to map these to the required exports (and root those when linking). But that requires complicated cross-language tooling interop - especially if the interop goes in both directions, where managed code trimming could reduce the set of used JS code.

I think the best solution for now is to require the user to manually preserve the required exports, for example using DynamicDependency in the user code module initializer, or using XML. If that's too much work, then the next best alternative is to preserve the whole interop assembly. I don't think it's right to do that via AssemblyMetadata("IsTrimmable", "false") - I think the decision whether to trim the interop assembly should belong to the application author.

[pavelsavara]

As Vitek said, it could get as bad as the whole assembly could be trimmed because of that. Trimming of System.Runtime.InteropServices.JavaScript assembly is just manifestation of the problem. We have the [DynamicDependency] on ValueType as a workaround (wasm only). We really need to keep this assembly anyway for wasm to work. For it the question is only how to do it, not if.

But user code could have the same issue, if they don't have application managed code reference to a assembly with JSExport whole MyJSHelper assembly could get trimmed. Most likely use case is, when we want to produce JS component in dotnet. Users of such component is not a dotnet app, but for example ReactJS app.

https://github.com/maraf/dotnet-wasm-react This ^^ sample is probably not trimmed only because we don't know how to produce library project without Main() yet.

The managed tooling doesn't have easy way how to know which [JSExport]ed method would be called from JS side. We are not going to be able to overcome that gap on JS side of the tooling, there are too many ways how the JS call could escape analysis, eval() being one of them.

So, I think that [JSExport] actually does mean, "please protect the method and the assembly". I think that user would have do that AssemblyMetadata("IsTrimmable", "false") in 99% cases themselves. And so, we could generate it on their behalf, if there is such AssemblyMetadata.

[vitek-karas]

Disabling trimming in the assembly is probably too much - all you need is to keep all the JSExport methods (and everything they depend on), the rest of the assembly should be trimmed as anything else.

I don't think we have anything in the linker which would do this currently. Even AssemblyMetadata("IsTrimmable", "false") is undefined as far as I know - https://github.com/dotnet/linker/issues/2991.

I still think that assemblies should not be able to "root" themselves in the app - it's kind of a bad precedent. But the only other approach would require some kind of build-time tool which scans assemblies for the JSExport attribute and adds them to the "Roots" some other way. Which is equally bad.

I think this is up to you @pavelsavara to define the desired behavior - is you think that the best approach is to automatically root all JSExport methods (and probably without a way for the user to revoke that decision), then we will need to introduce some new feature into the linker to do this - either https://github.com/dotnet/linker/issues/2991, or something else.

[pavelsavara]

Let pause and have this issue collect community feedback for few months. I have no data if it really happens in user apps.

[pavelsavara]

How does the [UnmanagedCallersOnly] deal with the same issue ? Or any [ComVisible(true)] export ?

[marek-safar]

Yep, I agree this is not JS problem only but AFAIK we don't have a reliable way how to keep method (or even field) even if it's not used beyond DynamicDependency which is not suitable here. Our workaround is not to trim user assembly at all where such scenario with JSExport occurs most often.

I'd also not mix this with the concept of how to trim libraries in general, which is a similar but different problem.

[vitek-karas]

How does the [UnmanagedCallersOnly] deal with the same issue ? Or any [ComVisible(true)] export ?

Currently native hosting and COM are not supported with trimming, so we didn't even try to solve this.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.

Issue Details

---

By removing global `[DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.JSFunctionBinding", "System.Runtime.InteropServices.JavaScript")] ` attribute from System.ValueType we could keep only marshaling support for types that are used. The attribute needs to be used today because there is no reference to the assembly from managed code.

Author:	marek-safar
Assignees:	kg, pavelsavara
Labels:	`arch-wasm`, `area-System.Runtime`, `size-reduction`
Milestone:	Future