Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: only use TCMALLOC when specified in build flags #1580

Open
251 opened this issue Mar 20, 2023 · 7 comments
Open

cmake: only use TCMALLOC when specified in build flags #1580

251 opened this issue Mar 20, 2023 · 7 comments

Comments

@251
Copy link
Contributor

251 commented Mar 20, 2023

With the latest change to the build system (#1569), TCMALLOC gets pulled in when it is available on the system. I'd prefer this to be an optional dependency that gets linked only when requested.

@ccadar
Copy link
Contributor

ccadar commented Mar 20, 2023

I think it should be the default, but it should be possible to turn it off. Is this not the case?

@251
Copy link
Contributor Author

251 commented Mar 20, 2023

It's just not what one would expect. (In other applications one has --enable-jemalloc, --enable-tcmalloc, ... flags.) Why would one default to a specific non-standard allocator? And what happens when we decide to support others in the future?

Is this not the case?

This should work when the flag is correctly implemented.

@ccadar
Copy link
Contributor

ccadar commented Mar 20, 2023

If TCMalloc is the best-performing allocator, it should be the default when available.
I am not sure what you mean by "what happens when we decide to support others in the future?" We would still have an ordered list of preferences, the same way we have one for constraint solvers for instance.

@251
Copy link
Contributor Author

251 commented Mar 20, 2023

If TCMalloc is the best-performing allocator

Based on what benchmarks and criteria and what versions of tcmalloc/glibc? E.g. our build scripts (and Ubuntu) still use the version from gperftools and not the latest from https://github.com/google/tcmalloc. Also, TCMALLOC can be annoying at first while debugging code or running KLEE with a sanitizer, e.g.:

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7f890e0c0672 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x7f890c82d032 in MallocExtension::Register(MallocExtension*) (/usr/lib/libtcmalloc.so.4+0x2d032)
    #2 0x7f890c813ab9  (/usr/lib/libtcmalloc.so.4+0x13ab9)
    #3 0x7f890e8a60fd  (/lib64/ld-linux-x86-64.so.2+0x50fd)

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).

I really think it's odd to pick up a non-standard allocator. What if I use a very slow instrumented version of TCMALLOC just for debugging purposes on my system?!

@ccadar
Copy link
Contributor

ccadar commented Mar 20, 2023

In my experience it works better. But I haven't run any systematic studies.
@MartinNowack @danielschemmel what is your opinion here?

@danielschemmel
Copy link
Contributor

If we are just talking about defaults, then KLEE should attempt for the optimal case given available resources. This is how KLEE deals with all other dependencies (e.g., we do not require --enable-z3, --enable-stp, etc).

This obviously leads to the question what we expect the optimal case to be.

  • tcmalloc used to require some additional memory (it was especially slow to release allocated memory back to the OS), but was faster than the glibc allocator. I have not checked benchmarks for some time, so it might be that either one has gotten better (or worse). See, for example the benchmark done for mimalloc
  • These benchmarks also show that tcmalloc is not the "best" allocator currently available.
  • tcmalloc causes minor problems with asan (which can be fixed with a suppression file), but has its own debugging opportunities, e.g., https://google.github.io/tcmalloc/gwp-asan.html . It is also easy enough to disable tcmalloc by default if asan is enabled.
  • If you use a very slow, instrumented version of anything as a system default, that is really on you?

Finally, I would like to propose that if tcmalloc is not expected to be better than the glibc allocator for a significant subset of use cases, we should completely remove support.

@251
Copy link
Contributor Author

251 commented Mar 20, 2023

but was faster than the glibc allocator

Many show improvements for heavily threaded code - we don't have many threads.

but has its own debugging opportunities

I'm pretty sure that's not in the old version we use (as mentioned above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants