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

[wasm] Improvements to startup performance of mono_wasm_get_assembly_exports #99924

Merged
merged 4 commits into from
Apr 5, 2024

Conversation

kg
Copy link
Contributor

@kg kg commented Mar 18, 2024

Generated JSImport/JSExport initializer does an Environment.Version check to see if we're on NET7. That check is tremendously expensive, because it fetches a CustomAttribute instance off of something in corlib, then parses the version string (parsing version strings initializes a bunch of stuff). This PR removes that check and adds a linker dependency to keep the registration method alive so it can be called when needed.

We will also need to document that DLLs (in nugets?) generated by this new version of the generator won't work on NET7 runtimes, since the relevant logic to perform automatic registration on that version.

@kg
Copy link
Contributor Author

kg commented Mar 18, 2024

Requesting review to get feedback. I hope someone knows trimming better than me and can suggest how to fix this. Put a dependency attribute of some kind on the generated module initializer, maybe?

@kg kg force-pushed the wasm-assemblyexports-opt-1 branch from 64759a1 to 8a4b7fc Compare April 3, 2024 19:36
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Apr 3, 2024
@kg kg force-pushed the wasm-assemblyexports-opt-1 branch from 8a4b7fc to 96666c5 Compare April 3, 2024 19:42
@kg kg removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 3, 2024
@kg kg requested a review from pavelsavara April 3, 2024 20:58
@pavelsavara
Copy link
Member

Could you please show the generated code ?

@kg
Copy link
Contributor Author

kg commented Apr 3, 2024

Could you please show the generated code ?

    [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, typeof(__GeneratedInitializer))]
        static internal void __Net7SelfInit_()
        {
        }

        [global::System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute("__Wrapper_PrepareToRender_1401412665", "MainJS", "RayTracer")]
        [global::System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute("__Wrapper_OnClick_1317452459", "MainJS", "RayTracer")]
        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("[RayTracer]MainJS:PrepareToRender", 1401412665, new global::System.Runtime.InteropServices.JavaScript.JSMarshalerType[] { global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.ArraySegment(global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Byte), global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Int32, global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Int32 });
            global::System.Runtime.InteropServices.JavaScript.JSFunctionBinding.BindManagedFunction("[RayTracer]MainJS:OnClick", 1317452459, new global::System.Runtime.InteropServices.JavaScript.JSMarshalerType[] { global::System.Runtime.InteropServices.JavaScript.JSMarshalerType.Task() });
        }
    }

@pavelsavara
Copy link
Member

Let's rename __Net7SelfInit_ to something like __DontTrimRegister_. Also please update description on top of this PR.

@kg kg force-pushed the wasm-assemblyexports-opt-1 branch from 9b8c760 to 77f918b Compare April 4, 2024 14:45
@pavelsavara
Copy link
Member

Here we are removing support for 3rd party library/nuget generated by Net9 SDK from being able to run on Net7 runtime.
I think it's OK. @lewing thoughts ?

@maraf
Copy link
Member

maraf commented Apr 5, 2024

Here we are removing support for 3rd party library/nuget generated by Net9 SDK from being able to run on Net7 runtime. I think it's OK. @lewing thoughts ?

.NET 7 is EOL on May 14, 2024. Removing support seems reasonable to me

@pavelsavara pavelsavara added the os-browser Browser variant of arch-wasm label Apr 5, 2024
@kg kg merged commit 0c92024 into dotnet:main Apr 5, 2024
94 of 97 checks passed
@radekdoulik
Copy link
Member

image works nicely

matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
…exports (dotnet#99924)

Change generated JSImport/JSExport initializer to not rely on Environment.Version, for faster startup
@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript linkable-framework Issues associated with delivering a linker friendly framework os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants