Skip to content

codegen: Mark resolved const GlobalRef as method root#60945

Merged
topolarity merged 1 commit intoJuliaLang:masterfrom
topolarity:ct/globalref-root
Feb 10, 2026
Merged

codegen: Mark resolved const GlobalRef as method root#60945
topolarity merged 1 commit intoJuliaLang:masterfrom
topolarity:ct/globalref-root

Conversation

@topolarity
Copy link
Member

@topolarity topolarity commented Feb 6, 2026

This allows --trim to safely* prune bindings without dropping the root required for the generated machine code.

Opening this PR with a "simple" solution to get numbers / opinions on the overhead of these extra roots.

The (significantly more complex) alternative will be to examine the bindings (back)edges during --trim serialization and reconstruct the relevant binding dependency edges at that time. I'll update this PR in-place with that solution if it seems like the right direction to go instead.

Resolves #60846.

* As a caveat, uninferred GlobalRefs can lead to missing bindings at runtime, but this is out-of-scope for --trim.

@topolarity topolarity added the needs tests Unit tests are required for this change label Feb 6, 2026
@oscardssmith oscardssmith added the trimming Issues with trimming functionality or PR's relevant to its performance/functionality label Feb 6, 2026
@nsajko nsajko added the bugfix This change fixes an existing bug label Feb 6, 2026
@gbaraldi
Copy link
Member

gbaraldi commented Feb 6, 2026

How was the code there not segfaulting? Because I thought in this case we just removed the globalref and got the underlying object but that was rooted by the code

@johroj
Copy link
Contributor

johroj commented Feb 6, 2026

How was the code there not segfaulting? Because I thought in this case we just removed the globalref and got the underlying object but that was rooted by the code

Are you referring to the MWE I posted? I think got some segfaults too while exploring, but usually it would halt before that with some other error message.

@topolarity
Copy link
Member Author

@topolarity
Copy link
Member Author

@Keno or @vtjnash any complaints about this solution?
Seems much simpler than trying to read the edges in staticdata

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

This seems like it should work, since it should shove it into the global dict eventually, which we then prune during sysimage generation

This allows `--trim` to safely* prune bindings without dropping
the root required for the generated machine code.

* uninferred GlobalRefs can lead to missing bindings at runtime,
  but these do not directly trigger UB / crash and they are
  relatively rare
@topolarity topolarity added merge me PR is reviewed. Merge when all tests are passing and removed needs tests Unit tests are required for this change labels Feb 9, 2026
@topolarity topolarity merged commit 484e7b1 into JuliaLang:master Feb 10, 2026
9 of 10 checks passed
@topolarity topolarity added backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 and removed merge me PR is reviewed. Merge when all tests are passing labels Feb 10, 2026
KristofferC pushed a commit that referenced this pull request Feb 25, 2026
This allows `--trim` to safely* prune bindings without dropping the root
required for the generated machine code.

The (significantly more complex) alternative will be to examine the
bindings (back)edges during `--trim` serialization and reconstruct the
relevant binding dependency edges at that time but that does
not seem worth it compared to the 23.6 KB overhead in the sysimage
to track these roots explicitly.

Resolves #60846.

\* As a caveat, uninferred GlobalRefs can lead to missing bindings at
runtime, but this is out-of-scope for --trim.

(cherry picked from commit 484e7b1)
@KristofferC KristofferC mentioned this pull request Feb 25, 2026
37 tasks
KristofferC pushed a commit that referenced this pull request Mar 3, 2026
This allows `--trim` to safely* prune bindings without dropping the root
required for the generated machine code.

The (significantly more complex) alternative will be to examine the
bindings (back)edges during `--trim` serialization and reconstruct the
relevant binding dependency edges at that time but that does
not seem worth it compared to the 23.6 KB overhead in the sysimage
to track these roots explicitly.

Resolves #60846.

\* As a caveat, uninferred GlobalRefs can lead to missing bindings at
runtime, but this is out-of-scope for --trim.

(cherry picked from commit 484e7b1)
@KristofferC KristofferC mentioned this pull request Mar 3, 2026
56 tasks
@KristofferC KristofferC removed the backport 1.13 Change should be backported to release-1.13 label Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.12 Change should be backported to release-1.12 bugfix This change fixes an existing bug trimming Issues with trimming functionality or PR's relevant to its performance/functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trimming: Elements added to global const become garbage collected

7 participants