From b75a14f2afb9ef336d7c7f7c35201316754a6ffa Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 11 Mar 2021 13:40:41 -0500 Subject: [PATCH 1/6] [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 --- src/mono/mono/metadata/metadata-update.c | 52 ++++++++++++++++++++++++ src/mono/mono/metadata/metadata-update.h | 13 ++++++ src/mono/mono/mini/interp/interp.c | 2 - src/mono/mono/mini/interp/transform.c | 13 ++++++ src/mono/wasm/runtime/driver.c | 6 --- 5 files changed, 78 insertions(+), 8 deletions(-) diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 12f25f8dabf25..9fed9981b4ee9 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -62,6 +62,58 @@ 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; +} + +/** + * 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; + /* FIXME: check the debugger bits on the MonoAssembly of the caller and callee */ + return TRUE; +} + static void mono_metadata_update_ee_init (MonoError *error); diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 59f041a7d3887..a8b5ee0d2b881 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); 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/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 From 74cdb31ad14a18a2b3f3c83768da3302fe14ec86 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 11 Mar 2021 13:45:07 -0500 Subject: [PATCH 2/6] [mbr] update samples --- src/mono/sample/mbr/browser/runtime.js | 2 +- src/mono/sample/mbr/console/Makefile | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) 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/Makefile b/src/mono/sample/mbr/console/Makefile index 34d57af739d05..ce1e540df671c 100644 --- a/src/mono/sample/mbr/console/Makefile +++ b/src/mono/sample/mbr/console/Makefile @@ -12,7 +12,7 @@ else TARGET_OS=linux endif -MONO_ENV_OPTIONS ?=--interp=-inline +MONO_ENV_OPTIONS = --interp publish: $(DOTNET) publish -c $(CONFIG) -r $(TARGET_OS)-$(MONO_ARCH) @@ -20,6 +20,7 @@ publish: 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: From ac7b264b9cec6ad9b8a9fc7626ea9a876fe24b73 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 11 Mar 2021 15:50:38 -0500 Subject: [PATCH 3/6] [mbr] Disable inlining for DebuggerAttribue assemblies with the optimizer disabled flag --- src/mono/mono/metadata/metadata-update.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 9fed9981b4ee9..c9e607cfaa010 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" @@ -110,8 +111,9 @@ mono_metadata_update_no_inline (MonoMethod *caller, MonoMethod *callee) { if (!mono_metadata_update_enabled (NULL)) return FALSE; - /* FIXME: check the debugger bits on the MonoAssembly of the caller and callee */ - return TRUE; + 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 From 7e901f830676999b87f1a1c0bee30d86522b298e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 11 Mar 2021 15:52:26 -0500 Subject: [PATCH 4/6] [mbr] Update samples --- src/mono/sample/mbr/DeltaHelper.targets | 1 + src/mono/sample/mbr/console/ConsoleDelta.csproj | 2 +- src/mono/sample/mbr/console/Makefile | 7 +++++-- 3 files changed, 7 insertions(+), 3 deletions(-) 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/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 ce1e540df671c..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) @@ -15,7 +18,7 @@ endif 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 \ From 1f5151df29f8a409723070b88a22ecd04c7a8d22 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 11 Mar 2021 16:06:12 -0500 Subject: [PATCH 5/6] [mbr] Throw InvalidOperationException is hot reload is not supported On a per-assembly basis --- src/mono/mono/metadata/metadata-update.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index c9e607cfaa010..510e7456ef722 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -95,6 +95,18 @@ mono_metadata_update_enabled (int *modifiable_assemblies_out) 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 @@ -935,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) From 458edc0b3f021a3b144e72c6858c424820573b9b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 11 Mar 2021 16:46:41 -0500 Subject: [PATCH 6/6] [mbr] Stub implementations if !ENABLE_METADATA_UPDATE --- src/mono/mono/metadata/metadata-update.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index a8b5ee0d2b881..961313b06e32b 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -57,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 */