Skip to content

[mono][interp] Add max limit to the imported IL when inlining #121580

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −0
Open

[mono][interp] Add max limit to the imported IL when inlining #121580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions src/mono/mono/mini/interp/transform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2990,6 +2990,8 @@ interp_get_icall_sig (MonoMethodSignature *sig)
/* larger than mono jit; chosen to ensure that List<T>.get_Item can be inlined */
#define INLINE_LENGTH_LIMIT 30
#define INLINE_DEPTH_LIMIT 10
// How much imported IL we allow before we stop inlining
#define INLINE_IL_BUDGET 1000000
Copy link
Member

@vitek-karas vitek-karas Nov 13, 2025

Choose a reason for hiding this comment

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

Can you check with the JIT team if they have a similar limit (I would expect so) and maybe we could put a similar value here.

Copy link
Member Author

@BrzVlad BrzVlad Nov 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I believe the jit has a more complex approach to inlining budget. From a quick look through the code they do a rough estimation based on the initial code size, then they have the max allowed increased in compilation time defined here https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/compiler.h#L11281, and for each method to inline they add the cost via this estimation https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/inline.cpp#L1097. (cc @AndyAyersMS in case I'm saying something wrong).

I didn't want to overly engineer this, since I'm not sure it is worth investing time into it, also risking some potential regressions. Also those numbers might be more specific to the RyuJIT and not apply ideally to the mono interpreter. This was just a simple and safe fix to prevent outliers that could prove problematic in production.

Copy link
Member

@vitek-karas vitek-karas Nov 13, 2025

Choose a reason for hiding this comment

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

Great - I agree that if the mechanism is not comparable, then let's stick with your value (the reasoning for it good)

Copy link
Member

@AndyAyersMS AndyAyersMS Nov 13, 2025

Choose a reason for hiding this comment

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

I don't think the RyuJit heuristic would generalize well ... also we are not super happy with it as it tends to give up on inlining too soon. What @BrzVlad proposes here looks sensible.


static gboolean
is_metadata_update_disabled (void)
Expand All @@ -3009,6 +3011,9 @@ interp_method_check_inlining (TransformData *td, MonoMethod *method, MonoMethodS
if (td->disable_inlining)
return FALSE;

if (td->total_il_size > INLINE_IL_BUDGET)
return FALSE;

// Exception handlers are always uncommon, with the exception of finally.
int inner_clause = td->clause_indexes [td->current_il_offset];
if (inner_clause != -1 && td->header->clauses [inner_clause].flags != MONO_EXCEPTION_CLAUSE_FINALLY)
Expand Down Expand Up @@ -3090,6 +3095,7 @@ interp_inline_method (TransformData *td, MonoMethod *target_method, MonoMethodHe
int *prev_in_offsets;
gboolean ret;
unsigned int prev_max_stack_height, prev_locals_size;
unsigned int prev_total_il_size;
int prev_n_data_items;
int i;
int prev_sp_offset;
Expand Down Expand Up @@ -3129,6 +3135,7 @@ interp_inline_method (TransformData *td, MonoMethod *target_method, MonoMethodHe
prev_aggressive_inlining = td->aggressive_inlining;
prev_imethod_items = td->imethod_items;
prev_has_inlined_one_call = td->has_inlined_one_call;
prev_total_il_size = td->total_il_size;
td->has_inlined_one_call = FALSE;
td->inlined_method = target_method;

Expand Down Expand Up @@ -3165,6 +3172,7 @@ interp_inline_method (TransformData *td, MonoMethod *target_method, MonoMethodHe
g_print ("Inline aborted method %s.%s\n", m_class_get_name (target_method->klass), target_method->name);
td->max_stack_height = prev_max_stack_height;
td->vars_size = prev_locals_size;
td->total_il_size = prev_total_il_size;

/* Remove any newly added items */
for (i = prev_n_data_items; i < td->n_data_items; i++) {
Expand Down Expand Up @@ -5353,6 +5361,8 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
td->in_start = td->ip = header->code;
end = td->ip + header->code_size;

td->total_il_size += header->code_size;

td->cbb = td->entry_bb = interp_alloc_bb (td);
td->entry_bb->emit_state = BB_STATE_EMITTING;

Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/interp/transform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ typedef struct
GPtrArray *relocs;
gboolean verbose_level;
GArray *line_numbers;
int total_il_size;
gboolean prof_coverage;
MonoProfilerCoverageInfo *coverage_info;
GList *dont_inline;
Expand Down
Loading