From d39ad7a8df5a7dc22e042a392919d181df34392d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Sat, 13 Mar 2021 13:22:31 -0500 Subject: [PATCH] [mono][mbr] Implement DOTNET_MODIFIABLE_ASSEMBLIES support (#49507) Implements https://github.com/dotnet/runtime/issues/47274 for Mono. 1. `DOTNET_MODIFIABLE_ASSEMBLIES` environment variable must be set to `debug` for hot reload to be enabled (in addition to the interpreter being enabled) 2. Assemblies must be compiled in Debug mode - we check `DebuggableAttribute` for the `DisableOptimizations` bit. If an assembly doesn't have the bit set, it can't be reloaded. 3. In the interpreter - don't try to inline when hot reload is enabled and the caller or callee has the DisableOptimizations bit set. * [mbr] Check DOTNET_MODIFIABLE_ASSEMBLIES for hot reload support If it's set to "debug" (case insensitive) then hot reload is enabled for assemblies compiled in debug mode. Otherwise hot reload is disabled * [mbr] Disable inlining for DebuggerAttribue assemblies with the optimizer disabled flag * [mbr] Update samples * [mbr] Throw InvalidOperationException is hot reload is not supported On a per-assembly basis * [mbr] Stub implementations if !ENABLE_METADATA_UPDATE --- src/mono/mono/metadata/metadata-update.c | 71 +++++++++++++++++++ src/mono/mono/metadata/metadata-update.h | 28 ++++++++ src/mono/mono/mini/interp/interp.c | 2 - src/mono/mono/mini/interp/transform.c | 13 ++++ src/mono/sample/mbr/DeltaHelper.targets | 1 + src/mono/sample/mbr/browser/runtime.js | 2 +- .../sample/mbr/console/ConsoleDelta.csproj | 2 +- src/mono/sample/mbr/console/Makefile | 10 ++- src/mono/wasm/runtime/driver.c | 6 -- 9 files changed, 122 insertions(+), 13 deletions(-) diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 12f25f8dabf25..510e7456ef722 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -13,6 +13,7 @@ #ifdef ENABLE_METADATA_UPDATE #include +#include "mono/metadata/assembly-internals.h" #include "mono/metadata/metadata-internals.h" #include "mono/metadata/metadata-update.h" #include "mono/metadata/object-internals.h" @@ -62,6 +63,71 @@ typedef struct _DeltaInfo { } DeltaInfo; +#define DOTNET_MODIFIABLE_ASSEMBLIES "DOTNET_MODIFIABLE_ASSEMBLIES" + +/** + * mono_metadata_update_enable: + * \param modifiable_assemblies_out: set to MonoModifiableAssemblies value + * + * Returns \c TRUE if metadata updates are enabled at runtime. False otherwise. + * + * If \p modifiable_assemblies_out is not \c NULL, it's set on return. + * + * The result depends on the value of the DOTNET_MODIFIABLE_ASSEMBLIES + * environment variable. "debug" means debuggable assemblies are modifiable, + * all other values are ignored and metadata updates are disabled. + */ +gboolean +mono_metadata_update_enabled (int *modifiable_assemblies_out) +{ + static gboolean inited = FALSE; + static int modifiable = MONO_MODIFIABLE_ASSM_NONE; + + if (!inited) { + char *val = g_getenv (DOTNET_MODIFIABLE_ASSEMBLIES); + if (!g_strcasecmp (val, "debug")) + modifiable = MONO_MODIFIABLE_ASSM_DEBUG; + g_free (val); + inited = TRUE; + } + if (modifiable_assemblies_out) + *modifiable_assemblies_out = modifiable; + return modifiable != MONO_MODIFIABLE_ASSM_NONE; +} + +static gboolean +assembly_update_supported (MonoAssembly *assm) +{ + int modifiable = 0; + if (!mono_metadata_update_enabled (&modifiable)) + return FALSE; + if (modifiable == MONO_MODIFIABLE_ASSM_DEBUG && + mono_assembly_is_jit_optimizer_disabled (assm)) + return TRUE; + return FALSE; +} + +/** + * mono_metadata_update_no_inline: + * \param caller: the calling method + * \param callee: the method being called + * + * Returns \c TRUE if \p callee should not be inlined into \p caller. + * + * If metadata updates are enabled either for the caller or callee's module, + * the callee should not be inlined. + * + */ +gboolean +mono_metadata_update_no_inline (MonoMethod *caller, MonoMethod *callee) +{ + if (!mono_metadata_update_enabled (NULL)) + return FALSE; + MonoAssembly *caller_assm = m_class_get_image(caller->klass)->assembly; + MonoAssembly *callee_assm = m_class_get_image(callee->klass)->assembly; + return mono_assembly_is_jit_optimizer_disabled (caller_assm) || mono_assembly_is_jit_optimizer_disabled (callee_assm); +} + static void mono_metadata_update_ee_init (MonoError *error); @@ -881,6 +947,11 @@ mono_image_load_enc_delta (MonoImage *image_base, gconstpointer dmeta_bytes, uin if (!is_ok (error)) return; + if (!assembly_update_supported (image_base->assembly)) { + mono_error_set_invalid_operation (error, "The assembly can not be edited or changed."); + return; + } + const char *basename = image_base->filename; /* FIXME: * (1) do we need to memcpy dmeta_bytes ? (maybe) diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 59f041a7d3887..961313b06e32b 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -11,6 +11,19 @@ #ifdef ENABLE_METADATA_UPDATE +enum MonoModifiableAssemblies { + /* modifiable assemblies are disabled */ + MONO_MODIFIABLE_ASSM_NONE = 0, + /* assemblies with the Debug flag are modifiable */ + MONO_MODIFIABLE_ASSM_DEBUG = 1, +}; + +gboolean +mono_metadata_update_enabled (int *modifiable_assemblies_out); + +gboolean +mono_metadata_update_no_inline (MonoMethod *caller, MonoMethod *callee); + void mono_metadata_update_init (void); @@ -44,6 +57,21 @@ mono_metadata_update_cleanup_on_close (MonoImage *base_image); MonoImage * mono_table_info_get_base_image (const MonoTableInfo *t); +#else /* ENABLE_METADATA_UPDATE */ + +static inline gboolean +mono_metadata_update_enabled (int *modifiable_assemblies_out) +{ + if (modifiable_assemblies_out) + *modifiable_assemblies_out = 0; + return FALSE; +} + +static inline gboolean +mono_metadata_update_no_inline (MonoMethod *caller, MonoMethod *callee) +{ + return FALSE; +} #endif /* ENABLE_METADATA_UPDATE */ diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 2c6bcc9e688c1..67c0cae5a59ed 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7230,8 +7230,6 @@ copy_imethod_for_frame (InterpFrame *frame) static void interp_metadata_update_init (MonoError *error) { - if ((mono_interp_opt & INTERP_OPT_INLINE) != 0) - mono_error_set_execution_engine (error, "Interpreter inlining must be turned off for metadata updates"); } #ifdef ENABLE_METADATA_UPDATE diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index f22cfb0f74723..276d65ad889ae 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2632,6 +2632,16 @@ interp_icall_op_for_sig (MonoMethodSignature *sig) #define INLINE_LENGTH_LIMIT 20 #define INLINE_DEPTH_LIMIT 10 +static gboolean +is_metadata_update_disabled (void) +{ + static gboolean disabled = FALSE; + if (disabled) + return disabled; + disabled = !mono_metadata_update_enabled (NULL); + return disabled; +} + static gboolean interp_method_check_inlining (TransformData *td, MonoMethod *method, MonoMethodSignature *csignature) { @@ -2686,6 +2696,9 @@ interp_method_check_inlining (TransformData *td, MonoMethod *method, MonoMethodS if (td->prof_coverage) return FALSE; + if (!is_metadata_update_disabled () && mono_metadata_update_no_inline (td->method, method)) + return FALSE; + if (g_list_find (td->dont_inline, method)) return FALSE; diff --git a/src/mono/sample/mbr/DeltaHelper.targets b/src/mono/sample/mbr/DeltaHelper.targets index f77caf5039426..faeb76fd01663 100644 --- a/src/mono/sample/mbr/DeltaHelper.targets +++ b/src/mono/sample/mbr/DeltaHelper.targets @@ -12,6 +12,7 @@ What other properties do I need to pass? Maybe hotreload-delta-gen should just expose an MSBuild task so we can pass everything --> $(HotreloadDeltaGenArgs) -p:Configuration=$(Configuration) $(HotreloadDeltaGenArgs) -p:RuntimeIdentifier=$(RuntimeIdentifier) + $(HotreloadDeltaGenArgs) -p:BuiltRuntimeConfiguration=$(BuiltRuntimeConfiguration) $(HotreloadDeltaGenArgs) -script:$(DeltaScript) diff --git a/src/mono/sample/mbr/browser/runtime.js b/src/mono/sample/mbr/browser/runtime.js index aa866c0b5dd7c..0856b8d0e0303 100644 --- a/src/mono/sample/mbr/browser/runtime.js +++ b/src/mono/sample/mbr/browser/runtime.js @@ -7,7 +7,7 @@ var Module = { App.init (); }; config.environment_variables = { - "MONO_METADATA_UPDATE": "1" + "DOTNET_MODIFIABLE_ASSEMBLIES": "debug" }; config.fetch_file_cb = function (asset) { return fetch (asset, { credentials: 'same-origin' }); diff --git a/src/mono/sample/mbr/console/ConsoleDelta.csproj b/src/mono/sample/mbr/console/ConsoleDelta.csproj index fb5886a37e0e6..04059b0272b7f 100644 --- a/src/mono/sample/mbr/console/ConsoleDelta.csproj +++ b/src/mono/sample/mbr/console/ConsoleDelta.csproj @@ -25,7 +25,7 @@ - $(ArtifactsBinDir)microsoft.netcore.app.runtime.$(RuntimeIdentifier)\$(Configuration) + $(ArtifactsBinDir)microsoft.netcore.app.runtime.$(RuntimeIdentifier)\$(BuiltRuntimeConfiguration) diff --git a/src/mono/sample/mbr/console/Makefile b/src/mono/sample/mbr/console/Makefile index 34d57af739d05..ee47d81f09b5c 100644 --- a/src/mono/sample/mbr/console/Makefile +++ b/src/mono/sample/mbr/console/Makefile @@ -2,7 +2,10 @@ TOP=../../../../../ DOTNET:=$(TOP)./dotnet.sh DOTNET_Q_ARGS=--nologo -v:q -consoleloggerparameters:NoSummary -CONFIG ?=Release +# How to build the project. For hot reload this must be Debug +CONFIG ?=Debug +# How was dotnet/runtime built? should be the same as build.sh -c option +BUILT_RUNTIME_CONFIG ?= Release MONO_ARCH=x64 OS := $(shell uname -s) @@ -12,14 +15,15 @@ else TARGET_OS=linux endif -MONO_ENV_OPTIONS ?=--interp=-inline +MONO_ENV_OPTIONS = --interp publish: - $(DOTNET) publish -c $(CONFIG) -r $(TARGET_OS)-$(MONO_ARCH) + $(DOTNET) publish -c $(CONFIG) -r $(TARGET_OS)-$(MONO_ARCH) -p:BuiltRuntimeConfiguration=$(BUILT_RUNTIME_CONFIG) run: publish COMPlus_DebugWriteToStdErr=1 \ MONO_ENV_OPTIONS="$(MONO_ENV_OPTIONS)" \ + DOTNET_MODIFIABLE_ASSEMBLIES=debug \ $(TOP)artifacts/bin/ConsoleDelta/$(MONO_ARCH)/$(CONFIG)/$(TARGET_OS)-$(MONO_ARCH)/publish/ConsoleDelta clean: diff --git a/src/mono/wasm/runtime/driver.c b/src/mono/wasm/runtime/driver.c index 052de799412dc..523e799573b8c 100644 --- a/src/mono/wasm/runtime/driver.c +++ b/src/mono/wasm/runtime/driver.c @@ -514,12 +514,6 @@ mono_wasm_load_runtime (const char *unused, int debug_level) #else mono_jit_set_aot_mode (MONO_AOT_MODE_INTERP_ONLY); -#ifdef ENABLE_METADATA_UPDATE - if (monoeg_g_hasenv ("MONO_METADATA_UPDATE")) { - interp_opts = "-inline"; - } -#endif - /* * debug_level > 0 enables debugging and sets the debug log level to debug_level * debug_level == 0 disables debugging and enables interpreter optimizations