incr.comp.: We don't achieve 100% CGU re-use when recompiling Stylo

[michaelwoerister]

The following CGU are re-compiled even though there's been no change to the source code (just touch components/style/lib.rs).

alloc-vec.volatile
style-properties-animated_properties
style-gecko_bindings-structs-root
style-properties-declaration_block
smallvec.volatile
style-gecko-wrapper
hashglobe-table.volatile
servo_arc.volatile
style-gecko_bindings-structs-root-mozilla.volatile
core-iter.volatile
style-values-computed.volatile
style-properties-longhands-grid_template_columns
style-properties-longhands-grid_template_rows
style-properties-longhands-border_image_source
style-properties-longhands-clip_path
style-properties-longhands-shape_outside
style-values-animated.volatile

Version of Stylo used: https://github.com/rust-lang-nursery/rustc-perf/tree/6b3404678be0c1e3c013ca9234083984ae13b101/collector/benchmarks/style-2f3bc0de49

Steps to reproduce:

cd collector/benchmarks/style-2f3bc0de49
cargo clean
cargo rustc --release --features gecko --manifest-path components/style/Cargo.toml -- --cap-lints=warn -Zincremental=/tmp/somethingsomething -Ztime-passes -Zhuman-readable-cgu-names
touch components/style/lib.rs
cargo rustc --release --features gecko --manifest-path components/style/Cargo.toml -- --cap-lints=warn -Zincremental=/tmp/somethingsomething -Ztime-passes -Zhuman-readable-cgu-names

rustc 1.25.0-nightly (b8398d947 2018-02-11)
binary: rustc
commit-hash: b8398d947d160ad4f26cc22da66e5fbc7030817b
commit-date: 2018-02-11
host: x86_64-unknown-linux-gnu
release: 1.25.0-nightly
LLVM version: 6.0

Need to investigate further why these CGUs are being recompiled? @SimonSapin, @emilio, is there maybe some kind of build script or proc-macro that modifies the source code during a rebuild?

[SimonSapin]

Adding -v to the commands I see that the build script is run the first time but not the second. As expected, since cargo:rerun-if-changed=… is used.

There are multiple (derive) proc macros in use, but they should be deterministic as far as I know.

[michaelwoerister]

Thanks for the information, @SimonSapin!

[michaelwoerister]

One way to find out if this is due to a StableHash instability is to compile the crate a few times in a row with the -Zincremental-verify-ich flag enabled. That should point to any instabilities pretty quickly.

[goffrie]

I suspect it is because of hashset ordering in style_derive; see in cg.rs

impl<'input, 'path> WhereClause<'input, 'path> {
    pub fn add_trait_bound(&mut self, ty: &Ty) {
        let trait_path = self.trait_path;
        let params = self.params;
        let mut found = self.trait_output.map(|_| HashSet::new());

.. later:

        if let Some(found) = found {
            for ident in found {
                let ty = Ty::Path(None, ident.into());

At least, this is responsible for causing differences in the --pretty=expanded output.

[michaelwoerister]

@goffrie Wow, great find! That surely would explain instabilities.

@SimonSapin What would you suggest that we do with the benchmark? Do newer versions of style_derive have this problem too? We could also just patch the benchmark. Anything that will make the generated AST deterministic would help.

[SimonSapin]

https://github.com/servo/servo/blob/master/components/style_derive/cg.rs now doesn’t use use HashSet at all, but that’s part of a large-ish change that’s not easily backported.

We can update the entire benchmark to current Servo, but maybe it’s undesirable to change what’s being measured too much?

Another option is patching the benchmark to replace HashSet with BTreeSet or using a non-random hasher, to get deterministic iteration order.

[michaelwoerister]

I'd say let's patch the benchmark. One thing to look out for is if the values of pointers are being hashed (or compared in the case of BTreeSet). The version of syn being used seems to be an old one so I didn't see immediately if that could be the case.

[andjo403]

looks like it is possible to close this

[michaelwoerister]

We should verify that we get 100% re-use first.

[wesleywiser]

Testing the latest version of style-servo with rustc 1.50.0-nightly (1c389ffef 2020-11-24) shows that we still aren't achieving 100% CGU reuse. After several builds, I'm still seeing CGUs being codegen'd again after touching lib.rs:

time: 0.000; rss: 1370MB        find_cgu_reuse
time: 0.212; rss: 1429MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.3)
time: 0.184; rss: 1501MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.15)
time: 0.152; rss: 1563MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.5)
time: 0.153; rss: 1612MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.9)
time: 0.076; rss: 1651MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.0)
time: 0.104; rss: 1680MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.6)
time: 0.156; rss: 1734MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.4)
time: 0.164; rss: 1760MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.1)
time: 0.166; rss: 1788MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.8)
time: 0.189; rss: 1821MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.10)
time: 0.173; rss: 1849MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.7)
time: 0.169; rss: 1889MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.14)
time: 1.741; rss: 1897MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.0)
time: 0.171; rss: 1921MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.13)
time: 0.138; rss: 1947MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.11)
time: 0.113; rss: 1948MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.2)
time: 4.121; rss: 1951MB        codegen_to_LLVM_IR
time: 0.000; rss: 1951MB        assert_dep_graph
time: 0.000; rss: 1951MB        serialize_dep_graph
time: 6.080; rss: 1951MB        codegen_crate
time: 0.099; rss: 1871MB        LLVM_module_optimize_function_passes(style.59jqlgid-cgu.12)
time: 0.194; rss: 1884MB        free_global_ctxt
time: 2.654; rss: 1890MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.6)
time: 1.109; rss: 1895MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.2)
time: 2.465; rss: 1895MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.8)
time: 2.680; rss: 1895MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.1)
time: 1.558; rss: 1882MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.12)
time: 1.989; rss: 1883MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.11)
time: 3.655; rss: 1884MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.4)
time: 5.152; rss: 1880MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.5)
time: 4.948; rss: 1874MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.9)
time: 3.449; rss: 1870MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.7)
time: 3.692; rss: 1870MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.10)
time: 3.506; rss: 1812MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.14)
time: 3.648; rss: 1806MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.13)
time: 6.908; rss: 1791MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.15)
time: 8.357; rss: 1661MB        LLVM_module_optimize_module_passes(style.59jqlgid-cgu.3)
time: 0.541; rss: 1993MB        LLVM_lto_optimize(style.59jqlgid-cgu.12)
time: 0.798; rss: 2000MB        LLVM_lto_optimize(style.59jqlgid-cgu.2)
time: 1.845; rss: 2008MB        LLVM_lto_optimize(style.59jqlgid-cgu.8)
time: 2.242; rss: 1997MB        LLVM_lto_optimize(style.59jqlgid-cgu.11)
time: 2.494; rss: 2003MB        LLVM_lto_optimize(style.59jqlgid-cgu.6)
time: 2.470; rss: 2003MB        LLVM_lto_optimize(style.59jqlgid-cgu.13)
time: 2.802; rss: 1999MB        LLVM_lto_optimize(style.59jqlgid-cgu.14)
time: 2.907; rss: 1999MB        LLVM_lto_optimize(style.59jqlgid-cgu.1)
time: 3.198; rss: 2006MB        LLVM_lto_optimize(style.59jqlgid-cgu.7)
time: 3.422; rss: 2011MB        LLVM_lto_optimize(style.59jqlgid-cgu.0)
time: 3.550; rss: 2019MB        LLVM_lto_optimize(style.59jqlgid-cgu.10)
time: 4.732; rss: 2078MB        LLVM_lto_optimize(style.59jqlgid-cgu.5)
time: 5.102; rss: 2102MB        LLVM_lto_optimize(style.59jqlgid-cgu.9)
time: 5.229; rss: 2110MB        LLVM_lto_optimize(style.59jqlgid-cgu.3)
time: 6.034; rss: 2145MB        LLVM_lto_optimize(style.59jqlgid-cgu.15)
time: 6.244; rss: 2147MB        LLVM_lto_optimize(style.59jqlgid-cgu.4)

[michaelwoerister]

I'm wondering why these CGUs are not named? Are you sure that this is an incr. build? You could try running the test again with -Zhuman-readable-cgu-names. That might give you some more insight.

[wesleywiser]

Doh! I forgot to pass -Cincremental. If I do that, I can confirm we get 100% CGU re-use. 🎉