T Compiler Meeting Agenda 2024 05 02

T-compiler Meeting Agenda 2024-05-02

Announcements

  • Today release of stable Rust 1.78 (blog post)
  • Next week it’s RustNL week: some? many? of us will be there. Should we skip next week’s meeting? Or maybe just a quick look if there’s anything urgent?
  • Reminder: if you see a PR/issue that seems like there might be legal implications due to copyright/IP/etc, please let us know (or at least message @davidtwco or @Wesley Wiser so we can pass it along).

Other WG meetings

MCPs/FCPs

WG checkins

  • @_WG-async by @nikomatsakis and @tmandry

    Checkin text

  • @_WG-diagnostics by @Esteban Küber and @oli

    Checkin text

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “Consider inner modules to be local in the non_local_definitions lint” rust#124539
    • Authored by @_urgau (thanks!)
    • Fixes #124396, a P-medium regression emitting a false positive and misleading error message when compiling the Diesel crate (comment):

      The wording [of the error message] might result in cases where the user will believe that this is an upstream diesel issue, rather than an issue with their local code.

  • No stable nominations for T-compiler this time.

T-types stable / T-types beta

  • No beta nominations for T-types this time.
  • No stable nominations for T-types this time.

PRs S-waiting-on-team

T-compiler

Issues of Note

Short Summary

P-critical

T-compiler

  • No P-critical issues for T-compiler this time.

T-types

  • No P-critical issues for T-types this time.

P-high regressions

P-high beta regressions

  • No P-high beta regressions this time.

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs for 2024-04-29

Several non-noise changes this week, with both improvements and regressions coming as a result. Overall compiler performance is roughly neutral across the week.

Triage done by @simulacrum. Revision range: a77f76e2..c65b2dc9

Summary:

(instructions:u) mean range count
Regressions (primary) 0.4% [0.2%, 1.4%] 104
Regressions (secondary) 2.4% [0.2%, 23.7%] 81
Improvements (primary) -3.8% [-26.1%, -0.3%] 10
Improvements (secondary) -1.6% [-4.6%, -0.5%] 12
All (primary) 0.1% [-26.1%, 1.4%] 114

2 Regressions, 2 Improvements, 3 Mixed; 1 of them in rollups 51 artifact comparisons made in total

Regressions

Use fulfillment in method probe, not evaluation #122317 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.2%, 1.3%] 38
Regressions (secondary) 4.2% [0.5%, 23.6%] 39
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.5% [0.2%, 1.3%] 38

Some additional attempts to fix perf were done in a follow-up PR (https://github.com/rust-lang/rust/pull/124303) but did not pan out to be successful. It looks like this is a correctness fix, so we’ll need to accept the regressions for now.

Stabilize Utf8Chunks #123909 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.2%, 1.1%] 11
Regressions (secondary) 0.8% [0.3%, 1.2%] 19
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.5% [0.2%, 1.1%] 11

The regressions are in doc builds, but are not really expected from what is a relatively small change. Further investigation feels warranted (see https://github.com/rust-lang/rust/pull/123909#issuecomment-2082668500).

Improvements

Rollup of 5 pull requests #124289 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.3% [-0.3%, -0.2%] 7
Improvements (secondary) -0.2% [-0.2%, -0.2%] 2
All (primary) -0.3% [-0.3%, -0.2%] 7

Unclear whether this is a genuine improvement. Performance seems to have re-regressed in #123126 (see Mixed results below), so this may just be bimodality of some kind.

Set writable and dead_on_unwind attributes for sret arguments #121298 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.5%, 0.5%] 1
Regressions (secondary) - - 0
Improvements (primary) -3.1% [-26.0%, -0.3%] 12
Improvements (secondary) -1.6% [-4.4%, -0.5%] 11
All (primary) -2.8% [-26.0%, 0.5%] 13

Mixed

Enable CrateNum query feeding via TyCtxt #123126 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.2%, 0.6%] 19
Regressions (secondary) 0.4% [0.3%, 0.5%] 5
Improvements (primary) - - 0
Improvements (secondary) -0.7% [-0.7%, -0.7%] 2
All (primary) 0.3% [0.2%, 0.6%] 19

This looks like it’s mostly just a regression to incremental. The PR description notes this is expected and sounds like there’s follow-up work planned (unclear whether it will help with performance though).

Stop using LLVM struct types for alloca #122053 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.2%, 0.3%] 8
Regressions (secondary) 0.4% [0.2%, 1.1%] 17
Improvements (primary) -1.9% [-1.9%, -1.9%] 1
Improvements (secondary) -0.4% [-0.4%, -0.4%] 1
All (primary) -0.0% [-1.9%, 0.3%] 9

Instruction counts are predominantly affected by some shuffling inside LLVM, but cycles are largely unaffected. See detailed analysis in https://github.com/rust-lang/rust/pull/122053#issuecomment-2074850501.

Abort a process when FD ownership is violated #124210 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.4%, 0.4%] 2
Regressions (secondary) - - 0
Improvements (primary) -0.8% [-0.8%, -0.8%] 1
Improvements (secondary) -0.1% [-0.1%, -0.1%] 2
All (primary) 0.0% [-0.8%, 0.4%] 3

Based on the self profile results I suspect this is caused by us generating more code in the downstream crate(s) as a result of the late-bound ub checks.

Nominated Issues

T-compiler

  • None

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

T-compiler

  • “Make create_def a side effect instead of marking the entire query as always red” rust#115613 (last review activity: 7 months ago)
    • cc: @cjgillot (pending rebase cc: @oli)
  • “tidy watcher” rust#114209 (last review activity: 4 months ago)
    • cc: @Wesley Wiser
  • “Remove unnecessary impl sorting in queries and metadata” rust#120812
    • cc: @cjgillot
  • “Use Box<[T]> for ProcessResult::Changed” rust#121355 (last review activity: 2 months ago)
    • cc: @Esteban Küber

Next week’s WG checkins

  • None

Next meetings' agenda draft: hackmd link