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.

Sign up for GitHub

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

Jump to bottom

[mjit] Initialize MonoVTable and MonoClass one level up #28

Merged
merged 2 commits into from
Jul 30, 2018
Merged

[mjit] Initialize MonoVTable and MonoClass one level up #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c926ca6
[MiniCompiler] Move icall to mini.c
luhenry Jul 26, 2018
67d935f
[mjit] Initialize MonoVTable and MonoClass one level up
luhenry Jul 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 69 additions & 9 deletions mono/mini/mini-runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ static GPtrArray *profile_options;
static GSList *tramp_infos;
GSList *mono_interp_only_classes;

static void register_icalls (void);
static void mini_runtime_register_icalls (void);

gboolean
mono_running_on_valgrind (void)
Expand Down Expand Up @@ -4418,7 +4418,8 @@ mini_init (const char *filename, const char *runtime_version)

mono_arch_register_lowlevel_calls ();

register_icalls ();
mini_runtime_register_icalls ();
mini_jit_register_icalls ();

mini_jit_register_icalls ();

Expand Down Expand Up @@ -4510,14 +4511,23 @@ mjit_initialize (void)
}

static gpointer
compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gint32 opt, MonoError *error)
mjit_compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gint32 opt, MonoError *error)
{
if (!g_hasenv("MONO_MJIT"))
return mono_jit_compile_method_inner (method, target_domain, opt, NULL, error);

static __thread gboolean is_mjit_compiling = FALSE;
MonoObject *compiler, *method_info, *ret;
MonoNativeCodeHandle native_code;
gpointer params[4];
gboolean bootstrapping;

bootstrapping = is_mjit_compiling;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused. is bootstrapping ever true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, when you are compiling the ICompiler.CompileMethod for example.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, it's still not obvious to me what happens here:

  • first call of mjit_compile_method_inner, let's say for method Foo () -> is_mjit_compiling = FALSE
  • bootstrapping = is_mjit_compiling = FALSE
  • since !bootstrapping, we set is_mjit_compiling = TRUE
  • bootstrapping is still FALSE, so we are using mono_runtime_invoke_checked ()
  • that means we end up in mjit_compile_method_inner again, this time with bootstrapping = is_mjit_compiling = TRUE, and therefore return a "fake compiled entry" of MethodInfo.ctor()/ICompiler.CompileMethod(). By "fake compiled entry" I mean a function pointer that calls into the interpreter again, and executes said method.
  • we are back to the first call of mjit_compile_method_inner and compile Foo() with the managed JIT (the managed JIT runs in the interpreter).
  • since !bootstrapping is still the case we unsert is_mjit_compiling when we are done for Foo().

So either:

  1. we anyway run the managed JIT always in the interpreter, then I don't get the point of this change, as it seems to be a complicated flow of things a lot with no change.
  2. Or, I'm missing something. Please explain 🙂

Copy link
Collaborator Author

@luhenry luhenry Jul 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@lewurm

  • first call of mjit_compile_method_inner, let's say for method Foo () -> is_mjit_compiling = FALSE
  • bootstrapping = is_mjit_compiling = FALSE
  • since !bootstrapping, we set is_mjit_compiling = TRUE
  • bootstrapping is still FALSE, so we are using mono_runtime_invoke_checked () to invoke MiniCompiler.CompileMethod(method = "Foo")
    • if we end up in mjit_compile_method_inner again, it means we are compiling MiniCompiler.CompileMethod (with MiniCompiler => bootstrapping) -> we invoke with the Interpreter so we don't end up in mjit_compile_method_inner again.
  • we are back to the first call of mjit_compile_method_inner and compile Foo() with the managed JIT which runs as JITted code because we compiled it while running in the Interpreter.
  • since !bootstrapping is still the case we unsert is_mjit_compiling when we are done for Foo().

Where this method needs improvement is in the case where we are trying to compile 1 of the method called in the call chain of MiniCompiler.CompileMethod, we will run the whole MiniCompiler.CompileMethod for that method in the Interpreter, even if we have some pieces already JITted. That's where your note on tiered compilation comes into play and it would make it work better, but that's not in the spectrum right now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

heh, missed that one bit. Thanks for being patient with me and taking time to explain it 🙂 very cool 👍


// g_printerr("%s: method = %s%s%s:%s (%p), bootstrapping = %s\n",
// __func__, (m_class_get_name_space(method->klass) && m_class_get_name_space(method->klass)[0] != '\0') ? m_class_get_name_space(method->klass) : "",
// (m_class_get_name_space(method->klass) && m_class_get_name_space(method->klass)[0] != '\0') ? "." : "", m_class_get_name (method->klass), method->name, method,
// bootstrapping ? "TRUE" : "FALSE");

if (!bootstrapping)
is_mjit_compiling = TRUE;

mono_lazy_initialize (&mjit_initialized, mjit_initialize);

Expand All @@ -4527,7 +4537,12 @@ compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gint32 opt,

/* Invoke methodInfo..ctor */
params [0] = &method;
mono_runtime_invoke_interpreter (MethodInfo_ctor_method, method_info, params, error);

if (bootstrapping)
mono_runtime_invoke_interpreter (MethodInfo_ctor_method, method_info, params, error);
else
mono_runtime_invoke_checked (MethodInfo_ctor_method, method_info, params, error);

return_val_if_nok (error, NULL);

/* Create compiler object */
Expand All @@ -4539,9 +4554,17 @@ compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gint32 opt,
params[1] = method_info;
params[2] = &opt;
params[3] = &native_code;
ret = mono_runtime_invoke_interpreter (ICompiler_CompileMethod_method, compiler, params, error);

if (bootstrapping)
ret = mono_runtime_invoke_interpreter (ICompiler_CompileMethod_method, compiler, params, error);
else
ret = mono_runtime_invoke_checked (ICompiler_CompileMethod_method, compiler, params, error);

return_val_if_nok (error, NULL);

if (!bootstrapping)
is_mjit_compiling = FALSE;

if (*(gint16*)mono_object_unbox (ret) != 0 /* CompilationResult.Ok */) {
/* set error */
return NULL;
Expand All @@ -4550,6 +4573,43 @@ compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gint32 opt,
return native_code.blob;
}

static gpointer
njit_compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gint32 opt, MonoError *error)
{
return mono_jit_compile_method_inner (method, target_domain, opt, NULL, error);
}

/*
* This function is to be used to call the compiler, and it needs be the single point of entry to the compiler.
*/
static gpointer
compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gint32 opt, MonoError *error)
{
MonoVTable *vtable;
gpointer res;

if (g_hasenv("MONO_MJIT"))
res = mjit_compile_method_inner (method, target_domain, opt, error);
else
res = njit_compile_method_inner (method, target_domain, opt, error);

if (res == NULL)
return NULL;

vtable = mono_class_vtable_checked (target_domain, method->klass, error);
return_val_if_nok (error, NULL);

if (method->wrapper_type != MONO_WRAPPER_REMOTING_INVOKE
&& method->wrapper_type != MONO_WRAPPER_REMOTING_INVOKE_WITH_CHECK
&& method->wrapper_type != MONO_WRAPPER_XDOMAIN_INVOKE)
{
mono_runtime_class_init_full (vtable, error);
return_val_if_nok (error, NULL);
}

return res;
}

typedef struct _InstalledRuntimeCode {
MonoJitInfo *jinfo;
} InstalledRuntimeCode;
Expand Down Expand Up @@ -4631,7 +4691,7 @@ ves_icall_mjit_execute_installed_method (InstalledRuntimeCode *irc, MonoArray *a
}

static void
register_icalls (void)
mini_runtime_register_icalls (void)
{
mono_add_internal_call ("System.Diagnostics.StackFrame::get_frame_info",
ves_icall_get_frame_info);
Expand Down
23 changes: 2 additions & 21 deletions mono/mini/mini.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4041,7 +4041,6 @@ mono_jit_compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gi
MonoCompile *cfg;
gpointer code = NULL;
MonoJitInfo *jinfo, *info;
MonoVTable *vtable;
MonoException *ex = NULL;
GTimer *jit_timer;
MonoMethod *prof_method, *shared;
Expand Down Expand Up @@ -4176,12 +4175,6 @@ mono_jit_compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gi
return NULL;
}

vtable = mono_class_vtable_checked (target_domain, method->klass, error);
if (!mono_error_ok (error)) {
mono_destroy_compile (cfg);
return NULL;
}

if (method->wrapper_type == MONO_WRAPPER_MANAGED_TO_NATIVE) {
if (mono_marshal_method_from_wrapper (method)) {
/* Native func wrappers have no method */
Expand All @@ -4193,24 +4186,12 @@ mono_jit_compile_method_inner (MonoMethod *method, MonoDomain *target_domain, gi
if (prof_method != method)
MONO_PROFILER_RAISE (jit_done, (prof_method, jinfo));

if (!(method->wrapper_type == MONO_WRAPPER_REMOTING_INVOKE ||
method->wrapper_type == MONO_WRAPPER_REMOTING_INVOKE_WITH_CHECK ||
method->wrapper_type == MONO_WRAPPER_XDOMAIN_INVOKE)) {
if (!mono_runtime_class_init_full (vtable, error)) {
mono_destroy_compile (cfg);
return NULL;
}
}

if (!code) {
mono_destroy_compile (cfg);
return NULL;
}

if (code_length)
*code_length = cfg->code_len;

mono_destroy_compile (cfg);

g_assert (code);
Copy link
Owner

Choose a reason for hiding this comment

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

Nice!

return code;
}

Expand Down