Skip to content

precompile_utils: Don't auto-enqueue macro methods for pre-compilation#57833

Merged
topolarity merged 1 commit intoJuliaLang:masterfrom
topolarity:ct/no-hint-macro-methods
Mar 26, 2025
Merged

precompile_utils: Don't auto-enqueue macro methods for pre-compilation#57833
topolarity merged 1 commit intoJuliaLang:masterfrom
topolarity:ct/no-hint-macro-methods

Conversation

@topolarity
Copy link
Copy Markdown
Member

@topolarity topolarity commented Mar 19, 2025 •
edited
Loading

Despite disabling these from being compiled in gf.c for dynamic invocations, the pre-compilation code was adding macro Methods anyway to the workqueue.

Replaces #57782

@topolarity topolarity added needs tests Unit tests are required for this change bugfix This change fixes an existing bug backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Mar 19, 2025
@topolarity topolarity requested a review from JeffBezanson March 19, 2025 22:29
@topolarity topolarity mentioned this pull request Mar 19, 2025
Closed
@topolarity topolarity changed the title precompile_utils: Don't enqueue macro methods for pre-compilation precompile_utils: Don't auto-enqueue macro methods for pre-compilation Mar 19, 2025
@topolarity
Copy link
Copy Markdown
Member Author

topolarity commented Mar 19, 2025 •
edited
Loading

@JeffBezanson What do you think about deleting the auto-nospecialize for macros in lowering?

Does it serve any purpose if we are generally not compiling this code anyways?

@topolarity topolarity removed the needs tests Unit tests are required for this change label Mar 20, 2025
@nsajko
Copy link
Copy Markdown
Member

nsajko commented Mar 20, 2025 •
edited
Loading

Decreases the invalidation count on running the following code from 214 to 163 🚀

struct I <: Integer end
Base.Int(::I) = 7
Base.UInt(::I) = 0x7

Data on each invalidation:

@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Mar 20, 2025 •
edited
Loading

Does these mean macro methods don't get precompiled at all? Isn't that desirable to avoid having to precompile them at every first use?

@JeffBezanson
Copy link
Copy Markdown
Member

JeffBezanson commented Mar 20, 2025 •
edited
Loading

Yes let's try removing the inserted nospecialize as an alternative to this and see how it does. If that doesn't help, we should find a way to implement the @ name policy in a single place so we are not playing whack-a-mole. For example, if the policy we want is not to infer them, that should maybe be implemented inside inference so we can still do the rest of precompilation for them.

@vtjnash
Copy link
Copy Markdown
Member

vtjnash commented Mar 20, 2025

macros do not get used at runtime, so including them in the precompile files is (mostly) a waste of space (as is specializing them)

vtjnash
vtjnash reviewed Mar 20, 2025
View reviewed changes
@KristofferC KristofferC mentioned this pull request Mar 20, 2025
Merged
@JeffBezanson
Copy link
Copy Markdown
Member

JeffBezanson commented Mar 20, 2025

OK, it's certainly possible that no noticeable latency improvements come from precompiling macros.

@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Mar 20, 2025 •
edited
Loading

macros do not get used at runtime, so including them in the precompile files is (mostly) a waste of space (as is specializing them)

They get used at runtime in scripts (notebooks etc) and by other packages during precompile time. I don't see why it doesn't make sense to compile e.g. the JuMP macros once instead of adding latency to every single script a user wants to run with it.

@nsajko
Copy link
Copy Markdown
Member

nsajko commented Mar 21, 2025

Would it make sense to resolve the tradeoff by:

  • doing the inference
  • preserving the results while the Julia process is running
  • not preserving the results in the precompile files?

@KristofferC KristofferC mentioned this pull request Mar 24, 2025
Merged
27 tasks
topolarity added a commit to topolarity/julia that referenced this pull request Mar 25, 2025
@topolarity
lowering: Don't implicitly @nospecialize macros
074ce42 
There's not much purpose of this de-specialization, since macros are
generally excluded from inference / compilation anyway (except for
JuliaLang#57833 which was a bug)

Since our AST is already relatively homogeneous, it's not clear that
we gained much by this de-specialization when it was used. Arbitrarily
de-specializing a function also often leads to bad (wide) edges in
inference etc., causing frequent invalidations.
topolarity added a commit to topolarity/julia that referenced this pull request Mar 25, 2025
@topolarity
lowering: Don't implicitly @nospecialize macros
8d1ce02 
There's not much purpose of this de-specialization, since macros are
generally excluded from inference / compilation anyway (except for
JuliaLang#57833 which was a bug)

Since our AST is already relatively homogeneous, it's not clear that
we gained much by this de-specialization when it was used. Arbitrarily
de-specializing a function also often leads to bad (wide) edges in
inference etc., causing frequent invalidations.
topolarity added a commit to topolarity/julia that referenced this pull request Mar 25, 2025
@topolarity
lowering: Don't implicitly @nospecialize macros
9637b0e 
There's not much purpose of this de-specialization, since macros are
generally excluded from inference / compilation anyway (except for
JuliaLang#57833 which was a bug)

Since our AST is already relatively homogeneous, it's not clear that
we gained much by this de-specialization when it was used. Arbitrarily
de-specializing a function also often leads to bad (wide) edges in
inference etc., causing frequent invalidations.
@topolarity
precompile_utils: Don't enqueue macro methods for pre-compilation
1a91678 
Despite disabling the runtime from compiling `macro` functions in
`gf.c`, the pre-compilation code was adding macro code anyway to
the workqueue.
@topolarity topolarity force-pushed the ct/no-hint-macro-methods branch from 511502c to 1a91678 Compare March 26, 2025 14:42
@topolarity topolarity merged commit 6ce51d3 into JuliaLang:master Mar 26, 2025
7 checks passed
nsajko added a commit to nsajko/julia that referenced this pull request Mar 27, 2025
@nsajko
adjust for recent changes on master (probably PR JuliaLang#57833)
a0e319a
KristofferC pushed a commit that referenced this pull request Mar 31, 2025
@topolarity @KristofferC
precompile_utils: Don't auto-enqueue macro methods for pre-compilat…
7b65e0b 
…ion (#57833)

Despite disabling these from being compiled in `gf.c` for dynamic
invocations, the pre-compilation code was adding `macro` Methods anyway
to the workqueue.

Replaces #57782

(cherry picked from commit 6ce51d3)
This was referenced Mar 31, 2025
Merged
Merged
KristofferC pushed a commit that referenced this pull request Mar 31, 2025
@topolarity @KristofferC
precompile_utils: Don't auto-enqueue macro methods for pre-compilat…
43ef67f 
…ion (#57833)

Despite disabling these from being compiled in `gf.c` for dynamic
invocations, the pre-compilation code was adding `macro` Methods anyway
to the workqueue.

Replaces #57782

(cherry picked from commit 6ce51d3)
KristofferC pushed a commit that referenced this pull request Mar 31, 2025
@topolarity @KristofferC
precompile_utils: Don't auto-enqueue macro methods for pre-compilat…
665cf7c 
…ion (#57833)

Despite disabling these from being compiled in `gf.c` for dynamic
invocations, the pre-compilation code was adding `macro` Methods anyway
to the workqueue.

Replaces #57782

(cherry picked from commit 6ce51d3)
topolarity added a commit that referenced this pull request Apr 1, 2025
@topolarity
precompile_utils: Don't auto-enqueue macro methods for pre-compilat…
75bf576 
…ion (#57833)

Despite disabling these from being compiled in `gf.c` for dynamic
invocations, the pre-compilation code was adding `macro` Methods anyway
to the workqueue.

Replaces #57782
@topolarity topolarity removed backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Apr 1, 2025
topolarity added a commit that referenced this pull request Apr 1, 2025
@topolarity
precompile_utils: Don't auto-enqueue macro methods for pre-compilat…
fab480d 
…ion (#57833)

Despite disabling these from being compiled in `gf.c` for dynamic
invocations, the pre-compilation code was adding `macro` Methods anyway
to the workqueue.

Replaces #57782
@topolarity topolarity removed the backport 1.10 Change should be backported to the 1.10 release label Apr 1, 2025
@KristofferC KristofferC mentioned this pull request Nov 20, 2025
Merged
@KristofferC KristofferC mentioned this pull request Dec 12, 2025
Merged
@KristofferC KristofferC mentioned this pull request Jan 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vtjnash vtjnash vtjnash approved these changes

@JeffBezanson JeffBezanson Awaiting requested review from JeffBezanson

Assignees

No one assigned

Labels

bugfix This change fixes an existing bug invalidations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@topolarity @nsajko @KristofferC @JeffBezanson @vtjnash