port:leveldb : minor upgrade with build system fixes and tcmalloc variant

[RJVB (René Bertin)]

This upgrade leveldb to the current git/head version 1.20.46 which gets rid of the need for patching the build system because the project has moved to using CMake.

Leveldb will make use of tcmalloc from the gperftools port when present, but I found out that this can have annoying side-effects. I've been seeing deadlocks for instance:

* thread #1: tid = 0x1105e90, 0x00007fff8f75eb16 libsystem_kernel.dylib`syscall_thread_switch + 10, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  * frame #0: 0x00007fff8f75eb16 libsystem_kernel.dylib`syscall_thread_switch + 10
    frame #1: 0x00007fff92529df6 libsystem_platform.dylib`_OSSpinLockLockSlow + 63
    frame #2: 0x00007fff8d86890d libsystem_pthread.dylib`_pthread_testcancel + 28
    frame #3: 0x00007fff99864d2e libsystem_c.dylib`nanosleep + 42
    frame #4: 0x000000010b75b7b1 libtcmalloc.4.dylib`base::internal::SpinLockDelay(int volatile*, int, int) + 122
    frame #5: 0x000000010b75b6af libtcmalloc.4.dylib`SpinLock::SlowLock() + 41
    frame #6: 0x000000010b74fe00 libtcmalloc.4.dylib`tcmalloc::CentralCacheLockAll() + 56
    frame #7: 0x00007fff97676cd1 libsystem_malloc.dylib`_malloc_fork_prepare + 49
    frame #8: 0x00007fff997fa161 libsystem_c.dylib`fork + 12
    frame #9: 0x0000000106be1c63 QtCore`forkfd(flags=<unavailable>, ppid=0x00007fff5c5e650c) + 483 at forkfd.c:691
    frame #10: 0x0000000106bd5801 QtCore`QProcessPrivate::startProcess(this=0x0000000129019100) + 3537 at qprocess_unix.cpp:471
    frame #11: 0x0000000106bd180f QtCore`QProcessPrivate::start(this=0x0000000129019100, mode=<unavailable>) + 431 at qprocess.cpp:2177
    frame #12: 0x0000000106bd14ca QtCore`QProcess::start(this=<unavailable>, program=<unavailable>, arguments=0x00000001289e5a28, mode=<unavailable>) + 154 at qprocess.cpp:2089
SNIP

Citing a Qt dev's assessment of this

This is the "atfork" code, the one that causes all malloc locks to be dropped  in the child process so that malloc() will work there regardless of the state  of the parent. The fact that it's deadlocking in the code that is supposed to  remove locks so that it won't deadlock is a good irony.

This could be a system bug triggered by the fact that linking to libtcmalloc.dylib replaces the system malloc routines, it could also be simply be the result of loading a plugin that uses leveldb and thus loads libtcmalloc. The gperftools instructions warn specifically against loading libtcmalloc dynamically because replacing the system malloc is safe only when done from the onset. It is NOT safe to manage allocated memory via both set of routines.

I think thus that it would be better to disable tcmalloc support in leveldb by default, and provide it as a build variant.

[ryandesign (Ryan Carsten Schmidt)]

I'm planning to wait for the next stable release before updating the port.

You said in your proposed variant description that "leveldb uses tcmalloc (but also all applications using leveldb)". So it sounds like this feature infects any other ports that might be built using leveldb, and therefore I will probably want to disable it unconditionally and not offer the user a way to opt into that infection.

[RJVB (René Bertin)]

I'd argue against that. Tcmalloc does indeed replace the system malloc but it's also a *lot* faster - which is why leveldb uses it in the first place. Currently leveldb is a dependency only of QtWebEngine and QtWebKit (AFAICT) and there it clearly doesn't pose a problem (supposing official builds of Chromium-based browsers use a standard leveldb build).

That's why I proposed a build variant and I've even hesitated making it the default because I have just no way of knowing if the deadlock I was seeing (NOT in an existing, unmodified port) wasn't in fact triggered by an issue in my code. WebEngine is slow enough as it is, so it would certainly benefit from any optimisation it can get. Has there ever been a report of problems caused by tcmalloc?

[ryandesign (Ryan Carsten Schmidt)]

I wasn't aware of the existence of tcmalloc or leveldb's use of it until you brought it up in this ticket.

I just don't want a port to have a variant that causes other ports that use that variant to build themselves differently. That's a pain to handle correctly, as the x11/quartz situation in MacPorts demonstrates.

[RJVB (René Bertin)]

leveldb has been depending on gperftools since the beginning, so you could have known :)

And the tcmalloc dependency is purely private to leveldb, there are no ABI differences, in fact, even the leveldb code doesn't contain anything that's tcmalloc-specific. My patch only disables the linking-in of tcmalloc; the current variant is no different than the optimisation (or debug!) variants other ports propose.

You can get back the "stock" leveldb behaviour by doing the equivalent of LD_PRELOAD=libtcmalloc.

Maybe read up a bit on the subject to make an informed decision? It'd be a pity to remove one of leveldb's sales arguments (its speed) just because it might cause side-effects. On my side I plan to sort this out further. Again, the issue might be caused by an issue in code I'm working on and/or a system bug in OS X 10.9 .

[catap (Kirill A. Korinsky)]

In 9a7a13c0a82e0ff23bc4e1d4335ef6c537f59a65/macports-ports (master):

leveldb: update to 1.23; claim maintainership

Closes: #56288

[cooljeanius (Eric Gallager)]

Replying to catap:

In 9a7a13c0a82e0ff23bc4e1d4335ef6c537f59a65/macports-ports (master):

leveldb: update to 1.23; claim maintainership

Closes: #56288

Pretty sure the conflict introduced here with gperftools should be a build-time one, not an install-time one. Also it's "tcmalloc" with a "c", not just "tmalloc".