Skip to content

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented Jan 22, 2022

Previously, rgctx entries where stored either in the class rgctx
or the method rgctx in linked structures, and accessed using
rgctx trampolines (for non-llvmonly configuration), or
inline code and fallback C code (for llvmonly configuration).
However, if a method has an mrgctx parameter, all the rgctx entries
can be stored as an array in the mrgctx and accessed using a simple
load.
One complication is that the mrgctx might need to be allocated before
the method is compiled/loaded, so the rgctx entries need to be stored
in a separate array, and the array needs to be initialized on
demand.

  • Add an 'entries' field to MonoMethodRuntimeGenericContext which stores
    the rgctx entries.
  • Add a MonoGSharedMethodInfo structure which stores the information
    needed to initialize a MonoMethodRuntimeGenericContext, i.e. the
    number of rgctx entries and their contents.
  • Add a MONO_PATCH_INFO_GSHARED_METHOD_INFO patch type to refer
    to the previous structure.
  • Add a mini_init_method_rgctx () JIT icall, which initializes
    an mrgctx if needed and generate code in the prolog of gshared methods
    to call it.

@ghost ghost assigned vargaz Jan 22, 2022
@ghost ghost added the area-VM-meta-mono label Jan 22, 2022
@vargaz vargaz force-pushed the gshared-new branch 7 times, most recently from 05a3821 to f366e2d Compare January 22, 2022 16:15
@vargaz
Copy link
Contributor Author

vargaz commented Jan 22, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Jan 22, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Jan 23, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz vargaz requested a review from EgorBo as a code owner January 23, 2022 20:29
@vargaz
Copy link
Contributor Author

vargaz commented Jan 23, 2022

These changes decrease the size of the wasm executable for the System.Runtime tests by about 8%.

@vargaz
Copy link
Contributor Author

vargaz commented Jan 23, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Jan 23, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Jan 29, 2022

The llvmfullaot lanes failures look related.

@vargaz
Copy link
Contributor Author

vargaz commented Feb 1, 2022

Can't reproduce the failure locally.

vargaz added 2 commits March 22, 2022 18:59
  Previously, rgctx entries where stored either in the class rgctx
or the method rgctx in linked structures, and accessed using
rgctx trampolines (for non-llvmonly configuration), or
inline code and fallback C code (for llvmonly configuration).
However, if a method has an mrgctx parameter, all the rgctx entries
can be stored as an array in the mrgctx and accessed using a simple
load.
One complication is that the mrgctx might need to be allocated before
the method is compiled/loaded, so the rgctx entries need to be stored
in a separate array, and the array needs to be initialized on
demand.

* Add an 'entries' field to MonoMethodRuntimeGenericContext which stores
the rgctx entries.
* Add a MonoGSharedMethodInfo structure which stores the information
needed to initialize a MonoMethodRuntimeGenericContext, i.e. the
number of rgctx entries and their contents.
* Add a MONO_PATCH_INFO_GSHARED_METHOD_INFO patch type to refer
to the previous structure.
* Add a mini_init_method_rgctx () JIT icall, which initializes
an mrgctx if needed and generate code in the prolog of gshared methods
to call it.
Previously, these methods were passed a vtable. Passing the mrgctx
instead simplifies some runtime code and allows smaller/faster access to
rgctx entries.
@vargaz
Copy link
Contributor Author

vargaz commented Mar 22, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Comment was made before the most recent commit for PR 64129 in repo dotnet/runtime

@vargaz
Copy link
Contributor Author

vargaz commented Mar 22, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Mar 23, 2022

The llvmfullaot lane failures are related, can't reproduce locally.

@vargaz
Copy link
Contributor Author

vargaz commented Mar 23, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Mar 23, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Mar 23, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Mar 23, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Mar 23, 2022

Failures are unrelated.

@vargaz vargaz merged commit 47c09fa into dotnet:main Mar 23, 2022
@vargaz vargaz deleted the gshared-new branch March 23, 2022 22:18

// The +1 is for the NULL check at the beginning
// FIXME: memory management
gpointer *entries = mono_mem_manager_alloc0 (get_default_mem_manager (), (sizeof (gpointer) * (info->num_entries + 1)));
Copy link
Contributor

Choose a reason for hiding this comment

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

why not fix it using the correct mem_manager, is this situation difficult to get it?

radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
* [mono] Optimize generic sharing for generic methods.

  Previously, rgctx entries where stored either in the class rgctx
or the method rgctx in linked structures, and accessed using
rgctx trampolines (for non-llvmonly configuration), or
inline code and fallback C code (for llvmonly configuration).
However, if a method has an mrgctx parameter, all the rgctx entries
can be stored as an array in the mrgctx and accessed using a simple
load.
One complication is that the mrgctx might need to be allocated before
the method is compiled/loaded, so the rgctx entries need to be stored
in a separate array, and the array needs to be initialized on
demand.

* Add an 'entries' field to MonoMethodRuntimeGenericContext which stores
the rgctx entries.
* Add a MonoGSharedMethodInfo structure which stores the information
needed to initialize a MonoMethodRuntimeGenericContext, i.e. the
number of rgctx entries and their contents.
* Add a MONO_PATCH_INFO_GSHARED_METHOD_INFO patch type to refer
to the previous structure.
* Add a mini_init_method_rgctx () JIT icall, which initializes
an mrgctx if needed and generate code in the prolog of gshared methods
to call it.

* [mono] Pass an mrgctx to static/valuetype gshared methods.

Previously, these methods were passed a vtable. Passing the mrgctx
instead simplifies some runtime code and allows smaller/faster access to
rgctx entries.

* Add rgctx trampolines in get_ftnptr_for_method ().

* [mono][wasm] Avoid AOTing Microsoft.CodeAnalysis.dll as well.
@ghost ghost locked as resolved and limited conversation to collaborators Apr 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants