[mono][interp] Add max limit to the imported IL when inlining #121580
Conversation
While we have some simple heuristics on whether we should inline a method or not, we have no limit on how many methods we can inline. So, if a method has 1000 callsites to methods that are inlineable, we will inline each one of them, significantly bloating the code. This is made worse by more advanced SSA optimizations, because the compilation time becomes very slow and uses a lot of memory. The limit is set as a simple upper limit on the total amount of IL code being imported as part of method compilation. This is set to a generous initial value of 1MB, since local testing showed that it is still handled decently well. The largest value obtained from running System.Runtime libtests suite is 100K. A customer provided sample was hitting 8MB of imported code, which was causing GBs of mem usage and several seconds to compile the method. In the end, all this code was discarded anyway, since it was exceeding interpreter offset limits, and we had to retry compilation with inlining disabled.
BrzVlad
requested review from
kotlarmilos,
steveisok and
vitek-karas
as code owners
|
Tagging subscribers to this area: @BrzVlad, @kotlarmilos |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a maximum limit on the total amount of IL code imported during method compilation in the Mono interpreter to prevent excessive inlining. Without this limit, methods with many inlineable callsites could result in extremely bloated code, causing significant memory usage and slow compilation times.
Key Changes:
- Introduces a 1MB budget (1,000,000 bytes) for total imported IL code during compilation
- Tracks accumulated IL size across all inlining operations
- Prevents further inlining once the budget is exceeded
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/mono/mono/mini/interp/transform.h |
Adds total_il_size field to TransformData struct to track cumulative imported IL |
src/mono/mono/mini/interp/transform.c |
Implements budget constant (1MB), budget check in inlining heuristic, IL size tracking in code generation, and proper save/restore of counter during inlining attempts |
| @@ -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 | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Great - I agree that if the mechanism is not comparable, then let's stick with your value (the reasoning for it good)
There was a problem hiding this comment.
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.
3 participants
While we have some simple heuristics on whether we should inline a method or not, we have no limit on how many methods we can inline. So, if a method has 1000 callsites to methods that are inlineable, we will inline each one of them, significantly bloating the code. This is made worse by more advanced SSA optimizations, because the compilation time becomes very slow and uses a lot of memory.
The limit is set as a simple upper limit on the total amount of IL code being imported as part of method compilation. This is set to a generous initial value of 1MB, since local testing showed that it is still handled decently well. The largest value obtained from running System.Runtime libtests suite is 100K. A customer provided sample was hitting 8MB of imported code, which was causing GBs of mem usage and several seconds to compile the method. In the end, all this code was discarded anyway, since it was exceeding interpreter offset limits, and we had to retry compilation with inlining disabled.