Because CGU merging relies on CGU sizes, but the CGU sizes before
inlining aren't accurate.
This requires tweaking how the sizes are updated during merging: if CGU
A and B both have an inlined function F, then `size(A + B)` will be a
little less than `size(A) + size(B)`, because `A + B` will only have one
copy of F. Also, the minimum CGU size is increased because it now has to
account for inlined functions.
This change doesn't have much effect on compile perf, but it makes
follow-on changes that involve more sophisticated reasoning about CGU
sizes much easier.
Remove `box_free` lang item
This PR removes the `box_free` lang item, replacing it with `Box`'s `Drop` impl. Box dropping is still slightly magic because the contained value is still dropped by the compiler.
Always put the `create_size_estimate` calls and `debug_dump` calls
within a timed scopes. This makes the four main steps look more similar
to each other.
The comment says "Find the smallest CGU that has exported symbols and
put the dead function stubs in that CGU". But the code sorts the CGUs by
size (smallest first) and then searches them in reverse order, which
means it will find the *largest* CGU that has exported symbols.
The erroneous code was introduced in #92142.
This commit changes it to use a simpler search, avoiding the sort, and
fixes the bug in the process.
Because tiny CGUs make compilation less efficient *and* result in worse
generated code.
We don't do this when the number of CGUs is explicitly given, because
there are times when the requested number is very important, as
described in some comments within the commit. So the commit also
introduces a `CodegenUnits` type that distinguishes between default
values and user-specified values.
This change has a roughly neutral effect on walltimes across the
rustc-perf benchmarks; there are some speedups and some slowdowns. But
it has significant wins for most other metrics on numerous benchmarks,
including instruction counts, cycles, binary size, and max-rss. It also
reduces parallelism, which is good for reducing jobserver competition
when multiple rustc processes are running at the same time. It's smaller
benchmarks that benefit the most; larger benchmarks already have CGUs
that are all larger than the minimum size.
Here are some example before/after CGU sizes for opt builds.
- html5ever
- CGUs: 16, mean size: 1196.1, sizes: [3908, 2992, 1706, 1652, 1572,
1136, 1045, 948, 946, 938, 579, 471, 443, 327, 286, 189]
- CGUs: 4, mean size: 4396.0, sizes: [6706, 3908, 3490, 3480]
- libc
- CGUs: 12, mean size: 35.3, sizes: [163, 93, 58, 53, 37, 8, 2 (x6)]
- CGUs: 1, mean size: 424.0, sizes: [424]
- tt-muncher
- CGUs: 5, mean size: 1819.4, sizes: [8508, 350, 198, 34, 7]
- CGUs: 1, mean size: 9075.0, sizes: [9075]
Note that CGUs of size 100,000+ aren't unusual in larger programs.
This loop is doing two different things. For inlined items, it's adding
them to the CGU. For all items, it's recording them in
`mono_item_placements`.
This commit splits it into two separate loops. This avoids putting root
mono items into `reachable`, and removes the low-value check that
`roots` doesn't contain inlined mono items.
Currently it sorts by symbol name, which is a mangled name like
`_ZN1a4main17hb29587cdb6db5f42E`, which leads to non-obvious orderings.
This commit changes it to use the existing
`items_in_deterministic_order`, which iterates in source code order.
I found this confusing because it includes the root item, plus the
inlined items reachable from the root item. The new formulation
separates the two parts more clearly.
Currently it overwrites all the CGUs with new CGUs. But those new CGUs
are just copies of the old CGUs, possibly with some things added. This
commit changes things so that each CGU just gets added to in place,
which makes things simpler and clearer.
It currently uses ranges, which index into `UsageMap::used_items`. This
commit changes it to just use `Vec`, which is much simpler to construct
and use. This change does result in more allocations, but it is few
enough that the perf impact is negligible.
`UsageMap` contains `used_map`, which maps from an item to the item it
uses. This commit add `user_map`, which is the inverse.
We already compute this inverse, but later on, and it is only held as a
local variable. Its simpler and nicer to put it next to `used_map`.
Currently, the code uses multiple words to describe when a mono item `f`
uses a mono item `g`, all of which have problems.
- `f` references `g`: confusing because there are multiple kinds of use,
e.g. "`f` calls `g`" is one, but "`f` takes a (`&T`-style) reference
of `g`" is another, and that's two subtly different meanings of
"reference" in play.
- `f` accesses `g`: meh, "accesses" makes me think of data, and this is
code.
- `g` is a neighbor (or neighbour) of `f`: is verbose, and doesn't
capture the directionality.
This commit changes the code to use "`f` uses `g`" everywhere. I think
it's better than the current terminology, and the consistency is
important.
Also, `InliningMap` is renamed `UsageMap` because (a) it was always
mostly about usage, and (b) the inlining information it did record was
removed in a recent commit.
We record inlining status for mono items in `MonoItems`, and then
transfer it to `InliningMap`, for later use in
`InliningMap::with_inlining_candidates`.
But we can just compute inlining status directly in
`InliningMap::with_inlining_candidates`, because the mono item is right
there. There's no need to compute it in advance.
This commit changes the code to do that, removing the need for
`MonoItems` and `InliningMap::inlines`. This does result in more calls
to `instantiation_mode` (one per static occurrence) but the performance
effect is negligible.
Remove `-Zcgu-partitioning-strategy`.
This option was introduced three years ago, but it's never been meaningfully used, and `default` is the only acceptable value.
Also, I think the `Partition` trait presents an interface that is too closely tied to the existing strategy and would probably be wrong for other strategies. (My rule of thumb is to not make something generic until there are at least two instances of it, to avoid this kind of problem.)
Also, I don't think providing multiple partitioning strategies to the user is a good idea, because the compiler already has enough obscure knobs.
This commit removes the option, along with the `Partition` trait, and the `Partitioner` and `DefaultPartitioning` types. I left the existing code in `compiler/rustc_monomorphize/src/partitioning/default.rs`, though I could be persuaded that moving it into
`compiler/rustc_monomorphize/src/partitioning/mod.rs` is better.
r? ``@wesleywiser``
This option was introduced three years ago, but it's never been
meaningfully used, and `default` is the only acceptable value.
Also, I think the `Partition` trait presents an interface that is too
closely tied to the existing strategy and would probably be wrong for
other strategies. (My rule of thumb is to not make something generic
until there are at least two instances of it, to avoid this kind of
problem.)
Also, I don't think providing multiple partitioning strategies to the
user is a good idea, because the compiler already has enough obscure
knobs.
This commit removes the option, along with the `Partition` trait, and
the `Partitioner` and `DefaultPartitioning` types. I left the existing
code in `compiler/rustc_monomorphize/src/partitioning/default.rs`,
though I could be persuaded that moving it into
`compiler/rustc_monomorphize/src/partitioning/mod.rs` is better.
I find that these structs obfuscate the code. Removing them and just
passing the individual fields around makes the `Partition` method
signatures a little longer, but makes the data flow much clearer. E.g.
- `codegen_units` is mutable all the way through.
- `codegen_units`'s length is changed by `merge_codegen_units`, but only
the individual elements are changed by `place_inlined_mono_items` and
`internalize_symbols`.
- `roots`, `internalization_candidates`, and `mono_item_placements` are
all immutable after creation, and all used by just one of the four
methods.
Three of the four methods in `DefaultPartitioning` are defined in
`default.rs`. But `merge_codegen_units` is defined in a separate module,
`merging`, even though it's less than 100 lines of code and roughly the
same size as the other three methods. (Also, the `merging` module
currently sits alongside `default`, when it should be a submodule of
`default`, adding to the confusion.)
In #74275 this explanation was given:
> I pulled this out into a separate module since it seemed like we might
> want a few different merge algorithms to choose from.
But in the three years since there have been no additional merging
algorithms, and there is no mechanism for choosing between different
merging algorithms. (There is a mechanism,
`-Zcgu-partitioning-strategy`, for choosing between different
partitioning strategies, but the merging algorithm is just one piece of
a partitioning strategy.)
This commit merges `merging` into `default`, making the code easier to
navigate and read.
- Pass a slice instead of an iterator to `debug_dump`.
- For each CGU set, print: the number of CGUs, the max and min size, and
the ratio of the max and min size (which indicates how evenly sized
they are).
- Print a `FINAL` entry, showing the absolute final results.
