[llvm] [InstrRef][nfc] Remove usage of unique_ptrs of arrays (PR #74203)
Felipe de Azevedo Piovezan via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 19 05:32:48 PST 2023
felipepiovezan wrote:
> max-rss is measured using `time -f "%M;%e"`, which should produce the same data as `time -l`. Note that this is only for the final linker invocation, so you'd want to run `ninja -v` to get the final `clang` invocation that performs LTO and run that under `time`. (For actual analysis, I would run under `massif`.)
>
> It's possible that there is some platform-specific difference here (libstdc++ vs libc++), but it seems unlikely that that this causes a 10% regression on Linux and no change on MacOS.
Are you sure `time -f %M` is reliable in the presence of a forking program like clang?
I used an Ubuntu VM and measured this with and without the patch and there is a ~5% difference:
```
166,580 -> 157,924
```
But then running this under `massif` I get no measured difference at all between runs and the peak memory usage is quite different from the `time` output, and the graph is _identical_ for both runs, which makes me think this is also not measuring the right process (even though the docs say "If your program forks, the child will inherit all the profiling data that has been gathered for the parent.").
```
Command: /home/build-user/llvm-project//build_RelWithDebInfo/bin/clang -O3 -fomit-frame-pointer -flto -DNDEBUG -g -fuse-ld=/home/build-user/llvm-project//build_RelWithDebInfo/bin/ld.lld CMakeFiles/pairlocalalign.dir/Calignm1.c.o CMakeFiles/pairlocalalign.dir/constants.c.o CMakeFiles/pairlocalalign.dir/defs.c.o CMakeFiles/pairlocalalign.dir/Falign.c.o CMakeFiles/pairlocalalign.dir/fft.c.o CMakeFiles/pairlocalalign.dir/fftFunctions.c.o CMakeFiles/pairlocalalign.dir/Galign11.c.o CMakeFiles/pairlocalalign.dir/genalign11.c.o CMakeFiles/pairlocalalign.dir/genGalign11.c.o CMakeFiles/pairlocalalign.dir/Halignmm.c.o CMakeFiles/pairlocalalign.dir/io.c.o CMakeFiles/pairlocalalign.dir/Lalign11.c.o CMakeFiles/pairlocalalign.dir/Lalignmm.c.o CMakeFiles/pairlocalalign.dir/mltaln9.c.o CMakeFiles/pairlocalalign.dir/MSalign11.c.o CMakeFiles/pairlocalalign.dir/MSalignmm.c.o CMakeFiles/pairlocalalign.dir/mtxutl.c.o CMakeFiles/pairlocalalign.dir/pairlocalalign.c.o CMakeFiles/pairlocalalign.dir/partQalignmm.c.o CMakeFiles/pairlocalalign.dir/partSalignmm.c.o CMakeFiles/pairlocalalign.dir/Qalignmm.c.o CMakeFiles/pairlocalalign.dir/Ralignmm.c.o CMakeFiles/pairlocalalign.dir/rna.c.o CMakeFiles/pairlocalalign.dir/SAalignmm.c.o CMakeFiles/pairlocalalign.dir/Salignmm.c.o CMakeFiles/pairlocalalign.dir/suboptalign11.c.o CMakeFiles/pairlocalalign.dir/tddis.c.o -o pairlocalalign -lm
Massif arguments: --time-unit=B --massif-out-file=/home/build-user/test-suite/stats_with_patch.massif.out
ms_print arguments: /home/build-user/test-suite/stats_with_patch.massif.out
--------------------------------------------------------------------------------
KB
436.1^ @@ :: ::
| #@ : @ :::
| @::::#@ ::: ::@ ::::: :
| @:@::: #@ : : : @ ::::: ::
| ::@:@::: #@ : : : @ ::::: :::
| : @:@::: #@ : : : @ ::::: :::::
| : @:@::: #@ : : : @ ::::: :::: :
| : ::::: @:@::: #@ : : : @ ::::: :::: ::::
| ::: : @:@::: #@ : : : @ ::::: :::: ::
| :::::: : @:@::: #@ : : : @ ::::: :::: :: @
| @: ::: : @:@::: #@ : : : @ ::::: :::: :: @:
| :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:::
| ::::@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :
| :::: :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :@
| @:: :: :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :@:::
| @:@:: :: :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :@
| ::@:@:: :: :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :@
| :: @:@:: :: :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :@ :
| :: @:@:: :: :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :@ :
| :: @:@:: :: :@: ::: : @:@::: #@ : : : @ ::::: :::: :: @:: :@ :
0 +----------------------------------------------------------------------->MB
0 1.203
Detailed snapshots: [3, 5, 11, 17, 19, 24 (peak), 25, 32, 45, 51, 61]
```
I also tried manually linking all the LTO object files with `llvm-link` and then running them through `llc`, and here I see a small memory difference. With patch:
```
MB
137.2^ #
| @@:#
| @@@::@@:#
| @ @@@@::@@:#
| @ @:@@@@::@@:#
| @::@:@@@@::@@:#
| @@::@:@@@@::@@:#
| ::@@@::@:@@@@::@@:#:
| :::: @@@::@:@@@@::@@:#:
| @:::::: @@@::@:@@@@::@@:#:
| @:::@: :::: @@@::@:@@@@::@@:#::
| @ :@:: @: :::: @@@::@:@@@@::@@:#::
| @:::::@:: @: :::: @@@::@:@@@@::@@:#::
| @@@::::::::::::: :::@:::::@:: @: :::: @@@::@:@@@@::@@:#::
| @ ::::::@@ :: : : :::: :::::@:::::@:: @: :::: @@@::@:@@@@::@@:#::
|:::::::@:::: : @@ :: : : :::: :::::@:::::@:: @: :::: @@@::@:@@@@::@@:#::
|:: ::::@:::: : @@ :: : : :::: :::::@:::::@:: @: :::: @@@::@:@@@@::@@:#::
|:: ::::@:::: : @@ :: : : :::: :::::@:::::@:: @: :::: @@@::@:@@@@::@@:#::
|:: ::::@:::: : @@ :: : : :::: :::::@:::::@:: @: :::: @@@::@:@@@@::@@:#::
|:: ::::@:::: : @@ :: : : :::: :::::@:::::@:: @: :::: @@@::@:@@@@::@@:#::
0 +----------------------------------------------------------------------->GB
0 7.103
```
Without patch:
```
MB
136.0^ #
| @ @@#
| @@@::@@#:
| @@:@@@::@@#:
| @@:@@@::@@#:
| :::@@:@@@::@@#:
| ::: @@:@@@::@@#:
| :@@::: @@:@@@::@@#:
| @:::::@ ::: @@:@@@::@@#::
| @::: :@ ::: @@:@@@::@@#::
| @::::@::: :@ ::: @@:@@@::@@#:::
| @ :@::: @::: :@ ::: @@:@@@::@@#:::
| @ ::::@::: @::: :@ ::: @@:@@@::@@#:::
| @::::@@::@::: :@:@::: ::@::: @::: :@ ::: @@:@@@::@@#:::
| : @::::::::@::: @ : @::::::@:@: : ::@::: @::: :@ ::: @@:@@@::@@#:::
|:::::::@:::: :: @::: @ : @:::: :@:@: : ::@::: @::: :@ ::: @@:@@@::@@#:::
|:: ::::@:::: :: @::: @ : @:::: :@:@: : ::@::: @::: :@ ::: @@:@@@::@@#:::
|:: ::::@:::: :: @::: @ : @:::: :@:@: : ::@::: @::: :@ ::: @@:@@@::@@#:::
|:: ::::@:::: :: @::: @ : @:::: :@:@: : ::@::: @::: :@ ::: @@:@@@::@@#:::
|:: ::::@:::: :: @::: @ : @:::: :@:@: : ::@::: @::: :@ ::: @@:@@@::@@#:::
0 +----------------------------------------------------------------------->GB
0 7.107
```
In these traces I see InstrRefBasedImpl.cpp show up quite a bit (unlike the clang ones). Also note how here the peak memory is around 100MB instead of KBs. The difference is quite small between the two.
https://github.com/llvm/llvm-project/pull/74203
More information about the llvm-commits
mailing list