Skip to content

Avoid inlining FnPtr::new in StructGenerator bindings - #566

Open
jamienicol wants to merge 2 commits into
rust-windowing:masterfrom
jamienicol:never-inline-fnptr-new
Open

Avoid inlining FnPtr::new in StructGenerator bindings#566
jamienicol wants to merge 2 commits into
rust-windowing:masterfrom
jamienicol:never-inline-fnptr-new

Conversation

@jamienicol

Copy link
Copy Markdown

This avoids pathologically slow compilation for ARMv7 targets with -Cprofile-generate.

Bindings emitted by StructGenerator call FnPtr::new hundreds of times. Under -Cprofile-generate, each inlined call adds profiling instrumentation to the same loader function. On ARMv7 targets, LLVM can then spend an unreasonable amount of time in the instruction scheduling stage of code generation for this function. In local testing, compilation remained in this stage after more than 90 minutes.

Preventing FnPtr::new from being inlined avoids this, restoring normal compilation times.

This avoids pathologically slow compilation for ARMv7 targets with
`-Cprofile-generate`.

Bindings emitted by StructGenerator call FnPtr::new hundreds of times.
Under `-Cprofile-generate`, each inlined call adds profiling
instrumentation to the same loader function. On ARMv7 targets, LLVM
can then spend an unreasonable amount of time in the instruction
scheduling stage of code generation for this function. In local
testing, compilation remained in this stage after more than 90
minutes.

Preventing FnPtr::new from being inlined avoids this, restoring normal
compilation times.
@jamienicol

jamienicol commented Aug 3, 2026

Copy link
Copy Markdown
Author

See servo/gleam#235 for the motivation behind this. Adding bindings for an additional extension increases -Cprofile-generate compilation times from ~2m (already much greater than the seconds for non-armv7 targets) to >90 mins. With this change it takes seconds again.

@jamienicol

Copy link
Copy Markdown
Author

The clippy warning seems unrelated to my change, but I pushed a second commit to resolve it anyway. Does this look okay to merge @MarijnS95 ?

I realise it's probably a rustc/llvm bug, but this repo does at least seem to have history of avoiding inlining for the sake of compilation times (e.g. #495)

jamienicol added a commit to jamienicol/firefox that referenced this pull request Aug 6, 2026
…. r?#gfx-reviewers

Update gleam to include servo/gleam#235,
adding bindings for ARB_base_instance/EXT_base_instance extensions.

Unfortunately this gleam PR cannot be merged until gl_generator merges
rust-windowing/gl-rs#566 and releases a new
version. Without the gl_generator fix, arm32 profile-generate builds
take hours due to what is presumably a rustc/LLVM bug.

In the meantime, use [patch.crates-io] to vendor specific git
revisions of gl_generator and gleam including the required fixes. Once
both the gl_generator and gleam pull requests have been merged and new
versions of the respective crates have been published, we should
switch back to using the crates.io versions.
jamienicol added a commit to jamienicol/firefox that referenced this pull request Aug 6, 2026
…. r?#gfx-reviewers

Update gleam to include servo/gleam#235,
adding bindings for ARB_base_instance/EXT_base_instance extensions.

Unfortunately this gleam PR cannot be merged until gl_generator merges
rust-windowing/gl-rs#566 and releases a new
version. Without the gl_generator fix, arm32 profile-generate builds
take hours due to what is presumably a rustc/LLVM bug.

In the meantime, use [patch.crates-io] to vendor specific git
revisions of gl_generator and gleam including the required fixes. Once
both the gl_generator and gleam pull requests have been merged and new
versions of the respective crates have been published, we should
switch back to using the crates.io versions.
jamienicol added a commit to jamienicol/firefox that referenced this pull request Aug 6, 2026
…. r?#gfx-reviewers

Update gleam to include servo/gleam#235,
adding bindings for ARB_base_instance/EXT_base_instance extensions.

Unfortunately this gleam PR cannot be merged until gl_generator merges
rust-windowing/gl-rs#566 and releases a new
version. Without the gl_generator fix, arm32 profile-generate builds
take hours due to what is presumably a rustc/LLVM bug.

In the meantime, use [patch.crates-io] to vendor specific git
revisions of gl_generator and gleam including the required fixes. Once
both the gl_generator and gleam pull requests have been merged and new
versions of the respective crates have been published, we should
switch back to using the crates.io versions.

Differential Revision: https://phabricator.services.mozilla.com/D316861
lando-worker Bot pushed a commit to mozilla-firefox/firefox that referenced this pull request Aug 7, 2026
…. r=gfx-reviewers,supply-chain-reviewers,ahale

Update gleam to include servo/gleam#235,
adding bindings for ARB_base_instance/EXT_base_instance extensions.

Unfortunately this gleam PR cannot be merged until gl_generator merges
rust-windowing/gl-rs#566 and releases a new
version. Without the gl_generator fix, arm32 profile-generate builds
take hours due to what is presumably a rustc/LLVM bug.

In the meantime, use [patch.crates-io] to vendor specific git
revisions of gl_generator and gleam including the required fixes. Once
both the gl_generator and gleam pull requests have been merged and new
versions of the respective crates have been published, we should
switch back to using the crates.io versions.

Differential Revision: https://phabricator.services.mozilla.com/D316861
github-actions Bot pushed a commit to servo/webrender that referenced this pull request Aug 9, 2026
…. r=gfx-reviewers,supply-chain-reviewers,ahale

Update gleam to include servo/gleam#235,
adding bindings for ARB_base_instance/EXT_base_instance extensions.

Unfortunately this gleam PR cannot be merged until gl_generator merges
rust-windowing/gl-rs#566 and releases a new
version. Without the gl_generator fix, arm32 profile-generate builds
take hours due to what is presumably a rustc/LLVM bug.

In the meantime, use [patch.crates-io] to vendor specific git
revisions of gl_generator and gleam including the required fixes. Once
both the gl_generator and gleam pull requests have been merged and new
versions of the respective crates have been published, we should
switch back to using the crates.io versions.

Differential Revision: https://phabricator.services.mozilla.com/D316861
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant