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

Restructure JSImport/JSExport generators to share more code and utilize more Microsoft.Interop.SourceGeneration shared code #107769

Merged
merged 25 commits into from
Feb 6, 2025

Conversation

jkoritzinsky
Copy link
Member

Split out the MarshalerType/Bind logic to not run through the generators (and instead run through the generator resolver only, which allows removing the IJSMarshallingGenerator type and many of the different generator types (as most of them end up using the same logic as the primitive generator).

Update JSFunctionBinding to not use Unsafe.AsPointer to allow the source generator to use collection expressions (fixing one of the rude-edit errors in #107577 by removing the stackalloc) without depending on Roslyn implementation details.

Also use more pattern matching instead of when expressions where possible in JSGeneratorResolver to make the giant table a little more readable in my opinion.

Replat the JS generators to use the same stub generators as the LibraryImport and GeneratedComInterface source generators by using local functions to shim from the JS generator signatures (which take a buffer) to the more standard "separate arguments" format used by those generators.

This PR also adds support in Microsoft.Interop.SourceGeneration for an exception marshaller that corresponds to a different native location than the return location (to support the separate exception return from the argument return).

…port and GeneratedComInterface by using local functions to shim from the JS generator signatures (which take a buffer) to the more standard "separate arguments" format used by those generators.

Split out the MarshalerType/Bind logic to not run through the generators, which allows removing the IJSMarshallingGenerator concept.
@jkoritzinsky jkoritzinsky force-pushed the js-generator-shared-code branch from 0bf3ad1 to 3cd828e Compare October 17, 2024 23:54
…return arguments before we do any marshalling (instead of after).
@jkoritzinsky
Copy link
Member Author

For return values, here's the old codegen for return types (to compare against the ValidateGeneratedSourceOutput_Return test)

Old Code Gen
// JSImports.g.cs
// <auto-generated/>
unsafe partial class Foo
{
    [global::System.Diagnostics.DebuggerNonUserCode]
    public static partial global::System.Threading.Tasks.Task<int> Func()
    {
        if (__signature_Func_408569705 == null)
        {
            __signature_Func_408569705 = global::System.Runtime.InteropServices.JavaScript.JSFunctionBinding.BindJSFunction("Func", null, new global::System.Runtime.InteropServices.JavaScript.JSMarshalerType[] { global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Task(global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Int32) });
        }

        global::System.Span<global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument> __arguments_buffer = stackalloc global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument[2];
        ref global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument __arg_exception = ref __arguments_buffer[0];
        __arg_exception.Initialize();
        ref global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument __arg_return = ref __arguments_buffer[1];
        __arg_return.Initialize();
        global::System.Threading.Tasks.Task<int> __retVal;
        global::System.Runtime.InteropServices.JavaScript.JSFunctionBinding.InvokeJS(__signature_Func_408569705, __arguments_buffer);
        // UnmarshalCapture - Capture the native data into marshaller instances in case conversion to managed data throws an exception.
        __arg_return.ToManaged(out __retVal, static (ref global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument __task_result_arg, out int __task_result) =>
        {
            __task_result_arg.ToManaged(out __task_result);
        });
        return __retVal;
    }

    static global::System.Runtime.InteropServices.JavaScript.JSFunctionBinding __signature_Func_408569705;
}
// JSExports.g.cs
// <auto-generated/>
namespace System.Runtime.InteropServices.JavaScript
{
    [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute]
    unsafe class __GeneratedInitializer
    {
        [global::System.ThreadStaticAttribute]
        static bool initialized;
        [global::System.Runtime.CompilerServices.ModuleInitializerAttribute, global::System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute(global::System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicMethods | global::System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicMethods, "System.Runtime.InteropServices.JavaScript.__GeneratedInitializer", "Console")]
        static internal void __TrimmingPreserve_()
        {
        }

        [global::System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute("__Wrapper_NetFunc_408569705", "Foo", "Console")]
        static void __Register_()
        {
            if (initialized || global::System.Runtime.InteropServices.RuntimeInformation.OSArchitecture != global::System.Runtime.InteropServices.Architecture.Wasm)
                return;
            initialized = true;
            global::System.Runtime.InteropServices.JavaScript.JSFunctionBinding.BindManagedFunction("[Console]Foo:NetFunc", 408569705, new global::System.Runtime.InteropServices.JavaScript.JSMarshalerType[] { global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Task(global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Int32) });
        }
    }
}
unsafe partial class Foo
{
    [global::System.Diagnostics.DebuggerNonUserCode]
    internal static unsafe void __Wrapper_NetFunc_408569705(global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument* __arguments_buffer)
    {
        ref global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument __arg_exception = ref __arguments_buffer[0];
        ref global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument __arg_return = ref __arguments_buffer[1];
        global::System.Threading.Tasks.Task<int> __retVal = default;
        try
        {
            __retVal = Foo.NetFunc();
            // Marshal - Convert managed data to native data.
            __arg_return.ToJS(__retVal, static (ref global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument __task_result_arg, int __task_result) =>
            {
                __task_result_arg.ToJS(__task_result);
            });
        }
        catch (global::System.Exception ex)
        {
            __arg_exception.ToJS(ex);
        }
    }
}

@jkoritzinsky
Copy link
Member Author

I ran the JSImportManyArgs benchmark and I saw the following results:

main: average 71.2133ms
pr: average 68.3718ms

So, this PR is also a perf improvement for JSImport, about 3ms, which is 4% faster.

__a15_native__js_arg.ToManaged(out a15, static (ref global::System.Runtime.InteropServices.JavaScript.JSMarshalerArgument __task_result_arg, out long __task_result) =>
object a1 = default;
long a2 = default;
long a3 = default;
Copy link
Member

Choose a reason for hiding this comment

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

These don't need to be assigned default

Copy link
Member Author

Choose a reason for hiding this comment

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

As we introduce exception handling here, we initialize the variables so (in case they're touched in the exception clause) they're initialized. At the time of generating this initialization, we can't know if they'll be used, only that we are generating a try block with some exception handling.

@jkoritzinsky
Copy link
Member Author

/ba-g failures are unrelated

@jkoritzinsky jkoritzinsky merged commit eadb357 into dotnet:main Feb 6, 2025
89 of 93 checks passed
@jkoritzinsky jkoritzinsky deleted the js-generator-shared-code branch February 6, 2025 02:01
grendello added a commit to grendello/runtime that referenced this pull request Feb 6, 2025
* main: (23 commits)
  add important remarks to NrbfDecoder (dotnet#111286)
  docs: fix spelling grammar and missing words in clr-code-guide.md (dotnet#112222)
  Consider type declaration order in MethodImpls (dotnet#111998)
  Add a feature flag to not use GVM in Linq Select (dotnet#109978)
  [cDAC] Implement ISOSDacInterface::GetMethodDescPtrFromIp (dotnet#110755)
  Restructure JSImport/JSExport generators to share more code and utilize more Microsoft.Interop.SourceGeneration shared code (dotnet#107769)
  Add more detailed explanations to control-flow RegexOpcode values (dotnet#112170)
  Add repo-specific condition to labeling workflows (dotnet#112169)
  Fix bad assembly when a nested exported type is marked via link.xml (dotnet#107945)
  Make `CalculateAssemblyAction` virtual. (dotnet#112154)
  JIT: Enable reusing profile-aware DFS trees between phases (dotnet#112198)
  Add support for LDAPTLS_CACERTDIR \ TrustedCertificateDirectory (dotnet#111877)
  JIT: Support custom `ClassLayout` instances with GC pointers in them (dotnet#112064)
  Factor positive lookaheads better into find optimizations (dotnet#112107)
  Add ImmutableCollectionsMarshal.AsMemory (dotnet#112177)
  [mono] ILStrip write directly to the output filestream (dotnet#112142)
  Allow the NativeAOT runtime pack to be specified as the ILC runtime package (dotnet#111876)
  JIT: some reworking for conditional escape analysis (dotnet#112194)
  Replace HELPER_METHOD_FRAME with DynamicHelperFrame in patchpoints (dotnet#112025)
  [Android] Decouple runtime initialization and entry point execution for Android sample (dotnet#111742)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime.InteropServices.JavaScript source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants