Page MenuHomePhabricator

[IR] AttrBuilder: change TargetDepAttrs to StringMap<SmallString<16>>
AbandonedPublic

Authored by lebedev.ri on Jun 6 2020, 3:10 PM.

Details

Summary

Looking at memory footprint of unity build of RawSpeed library
(-O3 -g0 -emit-llvm -Xclang -disable-llvm-optzns),
surprisingly AttrBuilder is top-1 in memory allocations,
which is not that surprising since it's a std::map<std::string, std::string>

Let's see what is the baseline memory footprint,
and what happens with just StringMap<std::string>/StringMap<SmallString<N>>:

$ heaptrack_print heaptrack.clang++.baseline.gz | tail -n 6 ; heaptrack_print heaptrack.clang++.proposed.gz | tail -n 6
total runtime: 11.76s.
calls to allocation functions: 2951292 (250981/s)
temporary memory allocations: 608175 (51719/s)
peak heap memory consumption: 231.36MB
peak RSS (including heaptrack overhead): 399.31MB
total memory leaked: 204.39MB

Migrating std::map<std::string, std::string> to StringMap<SmallString<16>> improves things:

total runtime: 11.33s.
calls to allocation functions: 2684647 (236908/s)
temporary memory allocations: 643713 (56804/s)
peak heap memory consumption: 231.44MB
peak RSS (including heaptrack overhead): 397.13MB
total memory leaked: 204.47MB

$ heaptrack_print -d heaptrack.clang++.baseline.gz heaptrack.clang++.proposed.gz | tail -n 6
total runtime: -0.43s.                            # -3.66 %
calls to allocation functions: -266645 (624461/s) # -9.03 % (!)
temporary memory allocations: 35538 (-83227/s)    # +5.84 % (:/)
peak heap memory consumption: 81.71KB
peak RSS (including heaptrack overhead): 0B
total memory leaked: 81.71KB

Diff Detail

Event Timeline

lebedev.ri created this revision.Jun 6 2020, 3:10 PM
lebedev.ri edited the summary of this revision. (Show Details)Jun 6 2020, 3:28 PM
lebedev.ri updated this revision to Diff 269043.Jun 7 2020, 3:24 AM

Actually ensure that check-llvm/check-clang passes.
Doesn't have any significant impact on results.

lebedev.ri edited the summary of this revision. (Show Details)Jun 7 2020, 8:19 AM
bkramer added inline comments.Jun 7 2020, 8:45 AM
llvm/include/llvm/IR/Attributes.h
727

I'm a bit confused that SmallString gives an improvement in the number of allocations. All modern standard libraries implement std::string with SSO, so std::string should behave identically to SmallString<16>. What standard library did you test this with?

870–871

I don't think this is used anywhere. Can we just delete it?

llvm/lib/IR/Attributes.cpp
888

Just sorting the string attributes would be sufficient. Sorting attributes is expensive.

lebedev.ri marked 5 inline comments as done.
lebedev.ri edited the summary of this revision. (Show Details)

Sort only string attributes.
@nikic how about this one?

llvm/include/llvm/IR/Attributes.h
727

I'm on debian sid, so there's really nothing weird going on here.

$ ldd bin/clang | grep c++
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fe8aa3a0000)
$ dpkg -S libstdc++.so
libx32stdc++6: /usr/libx32/libstdc++.so.6.0.28
libstdc++6:amd64: /usr/share/gdb/auto-load/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28-gdb.py
libx32stdc++6: /usr/libx32/libstdc++.so.6
libx32stdc++6: /usr/share/gdb/auto-load/usr/libx32/libstdc++.so.6.0.28-gdb.py
lib32stdc++6: /usr/lib32/libstdc++.so.6.0.28
libstdc++-10-dev:amd64: /usr/lib/gcc/x86_64-linux-gnu/10/libstdc++.so
libstdc++-8-dev:amd64: /usr/lib/gcc/x86_64-linux-gnu/8/libstdc++.so
libstdc++-9-dev:amd64: /usr/lib/gcc/x86_64-linux-gnu/9/libstdc++.so
libstdc++6:amd64: /usr/lib/x86_64-linux-gnu/libstdc++.so.6
lib32stdc++6: /usr/lib32/libstdc++.so.6
lib32stdc++6: /usr/share/gdb/auto-load/usr/lib32/libstdc++.so.6.0.28-gdb.py
libstdc++6:amd64: /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28
$ apt-cache show libstdc++6:amd64
Package: libstdc++6
Source: gcc-10
Version: 10.1.0-3
Installed-Size: 2351
Maintainer: Debian GCC Maintainers <debian-gcc@lists.debian.org>
Architecture: amd64
Replaces: libstdc++6-10-dbg (<< 4.9.0-3)
Depends: gcc-10-base (= 10.1.0-3), libc6 (>= 2.18), libgcc-s1 (>= 4.2)
Conflicts: scim (<< 1.4.2-1)
Breaks: gcc-4.3 (<< 4.3.6-1), gcc-4.4 (<< 4.4.6-4), gcc-4.5 (<< 4.5.3-2)
Description-en: GNU Standard C++ Library v3
 This package contains an additional runtime library for C++ programs
 built with the GNU compiler.
 .
 libstdc++-v3 is a complete rewrite from the previous libstdc++-v2, which
 was included up to g++-2.95. The first version of libstdc++-v3 appeared
 in g++-3.0.
Description-md5: 724ab84919e0e220afb960e36463914d
Multi-Arch: same
Homepage: http://gcc.gnu.org/
Tag: role::shared-lib
Section: libs
Priority: optional
Filename: pool/main/g/gcc-10/libstdc++6_10.1.0-3_amd64.deb
Size: 491824
MD5sum: b3c01846fda0d56a0ec15e24935a0da6
SHA256: 7349532219fbc71a211954dc0a7ca136343c14769cb71c9c4cabe032d6cc9c38

All modern standard libraries implement std::string with SSO, so std::string should behave identically to SmallString<16>.

I see:

      enum { _S_local_capacity = 15 / sizeof(_CharT) };

      union
      {
	_CharT           _M_local_buf[_S_local_capacity + 1];
	size_type        _M_allocated_capacity;
      };

So i would say it's one char less.
Also, here

static_assert(sizeof(SmallString<16>) == 40);
static_assert(sizeof(std::string) == 32);

which means that given same buffer size, we'll fit less SmallStrings in it,
causing new allocation sooner. How does StringMap grow?
I'd guess by consuming memory more aggressively, we end up growing less times.
At least that would be my guess.

llvm/lib/IR/Attributes.cpp
888

Right.

lebedev.ri abandoned this revision.Jun 7 2020, 12:04 PM
lebedev.ri marked an inline comment as done.

Let's not bother.

llvm/lib/IR/Attributes.cpp
884–886

Hm, that doesn't do it.