Skip to content

Conversation

sjkelly
Copy link
Contributor

@sjkelly sjkelly commented Mar 20, 2024

This helps to minimize the amount of missed precompiles
that occur by eliding the inlining cost check when julia is
run under --compile=all. This will lead to
slightly larger sysimage sizes. However, in practice it leads
to more extensive and successful precompilation, which can
minimize the number of JIT events in
call sites with dynamic dispatch.

This is an alternative to #53547, where removing the inlining
cost check was made universal. However, that lead to ~15% larger
sysimage sizes by default. This implements Jeff's suggestion that
this mode be enabled under --compile=all.

This helps to minimize the amount of missed precompiles
that occur by eliding the inlining cost check when julia is
run under `--compile=all`. This will lead to
slightly larger sysimage sizes. However, in practice it leads
to more extensive and successful precompilation, which can
minimize the number of JIT events in
call sites with dynamic dispatch.

This is an alternative to JuliaLang#53547, where removing the inlining
cost check was made universal. However, that lead to ~15% larger
sysimage sizes by default. This implements Jeff's  suggestion that
this mode be enabled under `--compile=all`.
@mbauman
Copy link
Member

mbauman commented Mar 25, 2024

It does seem like there's something going awry with the logic in the default case, but this looks to be a very reasonable solution. This is impacting a real-world use-case where we need to ensure 100% precompilation.

@vtjnash vtjnash merged commit f259eba into JuliaLang:master Apr 16, 2024
@vtjnash
Copy link
Member

vtjnash commented Apr 16, 2024

I think this is a clear W, since it is compile=all mode and already making a massive sysimage, so we shouldn't then exclude small things

if (inferred &&
inferred != jl_nothing &&
(jl_ir_inlining_cost(inferred) == UINT16_MAX)) {
(jl_options.compile_enabled != JL_OPTIONS_COMPILE_ALL && jl_ir_inlining_cost(inferred) == UINT16_MAX)) {
Copy link
Member

Choose a reason for hiding this comment

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

did you mean to write jl_options.compile_enabled == JL_OPTIONS_COMPILE_ALL || here instead?

Copy link
Member

Choose a reason for hiding this comment

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

Oooooh, good catch. Yes, I do believe that's what we intended. This was meant to be the "softer" #53547, where we only considered the inlineability heuristics when we're not compile_all'ing... intending to otherwise compile even small functions.

Copy link
Member

Choose a reason for hiding this comment

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

(For posterity: Fixed in 59d74d4.)

kpamnany pushed a commit to RelationalAI/julia that referenced this pull request May 15, 2025
…ang#53798)

This helps to minimize the amount of missed precompiles
that occur by eliding the inlining cost check when julia is
run under `--compile=all`. This will lead to
slightly larger sysimage sizes. However, in practice it leads
to more extensive and successful precompilation, which can
minimize the number of JIT events in
call sites with dynamic dispatch.

This is an alternative to JuliaLang#53547, where removing the inlining
cost check was made universal. However, that lead to ~15% larger
sysimage sizes by default. This implements Jeff's  suggestion that
this mode be enabled under `--compile=all`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants