Use codegen-units=1 in official builds. [chromium/src : main]

Sett 6 ganger
Hopp til første uleste melding

danakj (Gerrit)

ulest,
1. mars 2022, 13:37:4901.03.2022
til rust...@chromium.org, Chromium LUCI CQ, Nico Weber, chromium...@chromium.org, Matthew Riley

Attention is currently required from: Nico Weber.

Patch set 1:Auto-Submit +1

View Change

1 comment:

  • Patchset:

    • Patch Set #1:

      I was learning about this on the weekend, so let's do it while it's fresh in my head

To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
Gerrit-Change-Number: 3498521
Gerrit-PatchSet: 1
Gerrit-Owner: danakj <dan...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: danakj <dan...@chromium.org>
Gerrit-CC: Matthew Riley <mat...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Tue, 01 Mar 2022 18:37:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

danakj (Gerrit)

ulest,
1. mars 2022, 13:37:5301.03.2022
til rust...@chromium.org

Attention is currently required from: Nico Weber.

danakj uploaded patch set #2 to this change.

View Change

Use codegen-units=1 in official builds.

The rust compiler does the best job of optimizing within a crate when
it compiles everything in a single unit. This is slower, so it's not
used by developers generally, but we should use it in official builds.

R=tha...@chromium.org

Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
Cq-Include-Trybots: luci.chromium.try:linux-rust-x64-rel,android-rust-arm-rel
---
M build/config/compiler/BUILD.gn
1 file changed, 19 insertions(+), 0 deletions(-)

To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
Gerrit-Change-Number: 3498521
Gerrit-PatchSet: 2
Gerrit-Owner: danakj <dan...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: danakj <dan...@chromium.org>
Gerrit-CC: Matthew Riley <mat...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-MessageType: newpatchset

Nico Weber (Gerrit)

ulest,
1. mars 2022, 13:51:0701.03.2022
til danakj, rust...@chromium.org, Nico Weber, Chromium LUCI CQ, chromium...@chromium.org, Matthew Riley

Attention is currently required from: danakj.

Patch set 2:Code-Review +1

View Change

2 comments:

  • Patchset:

    • Patch Set #2:

      looks fine.

      i wonder if this actually helps much when we do LTO though.

  • File build/config/compiler/BUILD.gn:

    • Patch Set #2, Line 837: if (!use_thin_lto) {

      unrelated, but we should only embed bitcode when using Chromium's rust toolchain, else it can't work. (that's one reason we're building that)

To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
Gerrit-Change-Number: 3498521
Gerrit-PatchSet: 2
Gerrit-Owner: danakj <dan...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: danakj <dan...@chromium.org>
Gerrit-CC: Matthew Riley <mat...@chromium.org>
Gerrit-Attention: danakj <dan...@chromium.org>
Gerrit-Comment-Date: Tue, 01 Mar 2022 18:50:52 +0000

danakj (Gerrit)

ulest,
1. mars 2022, 13:55:4001.03.2022
til rust...@chromium.org, Nico Weber, Chromium LUCI CQ, chromium...@chromium.org, Matthew Riley

Attention is currently required from: Nico Weber.

Patch set 2:Auto-Submit +1Commit-Queue +1

View Change

2 comments:

  • Patchset:

    • Patch Set #2:

      looks fine. […]

      It helps with other optimizations as well beyond what LTO will do, I believe. Like general compiler optimization.

  • File build/config/compiler/BUILD.gn:

    • unrelated, but we should only embed bitcode when using Chromium's rust toolchain, else it can't work […]

      Will send another CL

To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
Gerrit-Change-Number: 3498521
Gerrit-PatchSet: 2
Gerrit-Owner: danakj <dan...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: danakj <dan...@chromium.org>
Gerrit-CC: Matthew Riley <mat...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Tue, 01 Mar 2022 18:55:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Weber <tha...@chromium.org>
Gerrit-MessageType: comment

danakj (Gerrit)

ulest,
1. mars 2022, 13:55:4601.03.2022
til rust...@chromium.org, Nico Weber, Chromium LUCI CQ, chromium...@chromium.org, Matthew Riley

Attention is currently required from: Nico Weber.

Patch set 2:Commit-Queue +2

View Change

    To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
    Gerrit-Change-Number: 3498521
    Gerrit-PatchSet: 2
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-Attention: Nico Weber <tha...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 18:55:36 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    danakj (Gerrit)

    ulest,
    1. mars 2022, 14:04:3101.03.2022
    til rust...@chromium.org, Nico Weber, Chromium LUCI CQ, chromium...@chromium.org, Matthew Riley

    View Change

    1 comment:

    • File build/config/compiler/BUILD.gn:

    To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
    Gerrit-Change-Number: 3498521
    Gerrit-PatchSet: 2
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 19:04:18 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: danakj <dan...@chromium.org>

    Nico Weber (Gerrit)

    ulest,
    1. mars 2022, 14:07:5601.03.2022
    til danakj, rust...@chromium.org, Nico Weber, Chromium LUCI CQ, chromium...@chromium.org, Matthew Riley

    Attention is currently required from: danakj.

    View Change

    1 comment:

    • Patchset:

      • Patch Set #2:

        It helps with other optimizations as well beyond what LTO will do, I believe. […]

        like what? the value is probably that it can optimize across all files in the crate since they go in a single o file, but LTO can optimize across .o files.

    To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
    Gerrit-Change-Number: 3498521
    Gerrit-PatchSet: 2
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-Attention: danakj <dan...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 19:07:45 +0000

    danakj (Gerrit)

    ulest,
    1. mars 2022, 14:11:5801.03.2022
    til rust...@chromium.org, Nico Weber, Chromium LUCI CQ, chromium...@chromium.org, Matthew Riley

    View Change

    1 comment:

    • Patchset:

      • Patch Set #2:

        like what? the value is probably that it can optimize across all files in the crate since they go in […]

        That's right the value proposition is that rustc sees all of the crate and generates a single .o file for it.

        But I thought LTO was more or less about inlining code and maybe I misunderstood that. I thought there's more information around at compile time or more possible optimizations, like loop unrolling. One benefit of codegen-units=1 is that all uses of foo<Bar>() get turned into a single function, but I think LTO should solve that too.

        FWIW rustc by default does LTO _inside_ a crate, while compiling with codegen-units=16 in release builds. But avoids LTO across crates because that's slow. This was explained in "Rust for Rustaceans" as a compromise, as codegen-units=16 speeds up the build but makes for worse code, and LTO inside the crate helps to mitigate that.

    To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
    Gerrit-Change-Number: 3498521
    Gerrit-PatchSet: 2
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 19:11:37 +0000

    Nico Weber (Gerrit)

    ulest,
    1. mars 2022, 15:08:2001.03.2022
    til danakj, rust...@chromium.org, Nico Weber, Chromium LUCI CQ, chromium...@chromium.org, Matthew Riley

    Attention is currently required from: danakj.

    View Change

    1 comment:

    • Patchset:

      • Patch Set #2:

        That's right the value proposition is that rustc sees all of the crate and generates a single . […]

        we use "thinlto" to make LTO less slow. as long as rustc can output .o files with bitcode, that should transparently work hopefully. (rustc shouldn't be involved in the link invocations, and lld will thinlto fine as long as it gets a .o file with bitcode in the right version.)

    To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
    Gerrit-Change-Number: 3498521
    Gerrit-PatchSet: 2
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-Attention: danakj <dan...@chromium.org>
    Gerrit-Comment-Date: Tue, 01 Mar 2022 20:08:10 +0000

    Chromium LUCI CQ (Gerrit)

    ulest,
    1. mars 2022, 15:19:4601.03.2022
    til danakj, rust...@chromium.org, Nico Weber, chromium...@chromium.org, Matthew Riley

    Chromium LUCI CQ submitted this change.

    View Change


    Approvals: Nico Weber: Looks good to me danakj: Commit; Send CL to CQ automatically after approval
    Use codegen-units=1 in official builds.

    The rust compiler does the best job of optimizing within a crate when
    it compiles everything in a single unit. This is slower, so it's not
    used by developers generally, but we should use it in official builds.

    R=tha...@chromium.org

    Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
    Cq-Include-Trybots: luci.chromium.try:linux-rust-x64-rel,android-rust-arm-rel
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3498521
    Reviewed-by: Nico Weber <tha...@chromium.org>
    Auto-Submit: danakj <dan...@chromium.org>
    Commit-Queue: danakj <dan...@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#976365}
    ---
    M build/config/compiler/BUILD.gn
    1 file changed, 24 insertions(+), 0 deletions(-)

    diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn
    index 7315319..9d4ab29 100644
    --- a/build/config/compiler/BUILD.gn
    +++ b/build/config/compiler/BUILD.gn
    @@ -838,6 +838,9 @@
    # Optimization - don't include bitcode if it won't be used.
    rustflags += [ "-Cembed-bitcode=no" ]
    }
    + if (is_official_build) {
    + rustflags += [ "-Ccodegen-units=1" ]
    + }
    }

    # The BUILDCONFIG file sets this config on targets by default, which means when

    To view, visit change 3498521. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Id12861ae7a390f051b4d3f1069c07f745ee1fcfa
    Gerrit-Change-Number: 3498521
    Gerrit-PatchSet: 3
    Gerrit-Owner: danakj <dan...@chromium.org>
    Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
    Gerrit-Reviewer: danakj <dan...@chromium.org>
    Gerrit-CC: Matthew Riley <mat...@chromium.org>
    Gerrit-MessageType: merged
    Svar alle
    Svar til forfatter
    Videresend
    0 nye meldinger