From 83aceb216c100fc3f15ed5e241684d0138516b59 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Wed, 15 Jun 2022 14:53:52 -0700 Subject: [PATCH] Use absl::GetStackTrace for slow task profiler (#7374) * Make SlowTask workload runnable in joshua * Remove SignalSafeUnwind, and use absl::GetStackTrace for slow task profiler --- .../workloads/SlowTaskWorkload.actor.cpp | 13 ++--- flow/CMakeLists.txt | 2 - flow/Net2.actor.cpp | 4 -- flow/Platform.actor.cpp | 2 +- flow/SignalSafeUnwind.cpp | 55 ------------------- flow/SignalSafeUnwind.h | 30 ---------- tests/CMakeLists.txt | 2 +- tests/{ => noSim}/SlowTask.txt | 1 + 8 files changed, 8 insertions(+), 101 deletions(-) delete mode 100644 flow/SignalSafeUnwind.cpp delete mode 100644 flow/SignalSafeUnwind.h rename tests/{ => noSim}/SlowTask.txt (82%) diff --git a/fdbserver/workloads/SlowTaskWorkload.actor.cpp b/fdbserver/workloads/SlowTaskWorkload.actor.cpp index 3eec25fdec..0fa5f5614c 100644 --- a/fdbserver/workloads/SlowTaskWorkload.actor.cpp +++ b/fdbserver/workloads/SlowTaskWorkload.actor.cpp @@ -22,7 +22,6 @@ #include "contrib/fmt-8.1.1/include/fmt/format.h" #include "fdbserver/workloads/workloads.actor.h" -#include "flow/SignalSafeUnwind.h" #include "flow/actorcompiler.h" // This must be the last #include. // Stress test the slow task profiler or flow profiler @@ -43,24 +42,22 @@ struct SlowTaskWorkload : TestWorkload { ACTOR static Future go() { wait(delay(1)); - int64_t phc = dl_iterate_phdr_calls; int64_t startProfilesDeferred = getNumProfilesDeferred(); int64_t startProfilesOverflowed = getNumProfilesOverflowed(); int64_t startProfilesCaptured = getNumProfilesCaptured(); int64_t exc = 0; - fprintf(stderr, "Slow task starting\n"); + fprintf(stdout, "Slow task starting\n"); for (int i = 0; i < 10; i++) { - fprintf(stderr, " %d\n", i); + fprintf(stdout, " %d\n", i); double end = timer() + 1; while (timer() < end) { do_slow_exception_thing(&exc); } } - fmt::print(stderr, - "Slow task complete: {0} exceptions; {1} calls to dl_iterate_phdr, {2}" - " profiles deferred, {3} profiles overflowed, {4} profiles captured\n", + fmt::print(stdout, + "Slow task complete: {0} exceptions; {1} profiles deferred, {2} profiles overflowed, {3} profiles " + "captured\n", exc, - dl_iterate_phdr_calls - phc, getNumProfilesDeferred() - startProfilesDeferred, getNumProfilesOverflowed() - startProfilesOverflowed, getNumProfilesCaptured() - startProfilesCaptured); diff --git a/flow/CMakeLists.txt b/flow/CMakeLists.txt index 615f4dd44a..84166b3d8a 100644 --- a/flow/CMakeLists.txt +++ b/flow/CMakeLists.txt @@ -62,8 +62,6 @@ set(FLOW_SRCS Profiler.h ScopeExit.h SendBufferIterator.h - SignalSafeUnwind.cpp - SignalSafeUnwind.h SimpleOpt.h StreamCipher.cpp StreamCipher.h diff --git a/flow/Net2.actor.cpp b/flow/Net2.actor.cpp index 6da1db90b4..8f09866baa 100644 --- a/flow/Net2.actor.cpp +++ b/flow/Net2.actor.cpp @@ -89,10 +89,6 @@ void initProfiling() { net2backtraces = new volatile void*[net2backtraces_max]; other_backtraces = new volatile void*[net2backtraces_max]; - // According to folk wisdom, calling this once before setting up the signal handler makes - // it async signal safe in practice :-/ - backtrace(const_cast(other_backtraces), net2backtraces_max); - sigemptyset(&sigprof_set); sigaddset(&sigprof_set, SIGPROF); } diff --git a/flow/Platform.actor.cpp b/flow/Platform.actor.cpp index a4882a3312..c916c30571 100644 --- a/flow/Platform.actor.cpp +++ b/flow/Platform.actor.cpp @@ -3738,7 +3738,7 @@ void profileHandler(int sig) { ps->timestamp = checkThreadTime.is_lock_free() ? checkThreadTime.load() : 0; // SOMEDAY: should we limit the maximum number of frames from backtrace beyond just available space? - size_t size = backtrace(ps->frames, net2backtraces_max - net2backtraces_offset - 2); + size_t size = platform::raw_backtrace(ps->frames, net2backtraces_max - net2backtraces_offset - 2); ps->length = size; diff --git a/flow/SignalSafeUnwind.cpp b/flow/SignalSafeUnwind.cpp deleted file mode 100644 index a43dd81277..0000000000 --- a/flow/SignalSafeUnwind.cpp +++ /dev/null @@ -1,55 +0,0 @@ -/* - * SignalSafeUnwind.cpp - * - * This source file is part of the FoundationDB open source project - * - * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "flow/SignalSafeUnwind.h" - -int64_t dl_iterate_phdr_calls = 0; - -#if defined(__linux__) && !defined(USE_SANITIZER) - -#include -#include - -static int (*chain_dl_iterate_phdr)(int (*callback)(struct dl_phdr_info* info, size_t size, void* data), - void* data) = nullptr; - -static void initChain() { - static std::once_flag flag; - - // Ensure that chain_dl_iterate_phdr points to the "real" function that we are overriding - std::call_once(flag, []() { *(void**)&chain_dl_iterate_phdr = dlsym(RTLD_NEXT, "dl_iterate_phdr"); }); - - if (!chain_dl_iterate_phdr) { - criticalError(FDB_EXIT_ERROR, "SignalSafeUnwindError", "Unable to find dl_iterate_phdr symbol"); - } -} - -// This overrides the function in libc! -extern "C" int dl_iterate_phdr(int (*callback)(struct dl_phdr_info* info, size_t size, void* data), void* data) { - interlockedIncrement64(&dl_iterate_phdr_calls); - - initChain(); - - setProfilingEnabled(0); - int result = chain_dl_iterate_phdr(callback, data); - setProfilingEnabled(1); - return result; -} -#endif \ No newline at end of file diff --git a/flow/SignalSafeUnwind.h b/flow/SignalSafeUnwind.h deleted file mode 100644 index 2d17b6637a..0000000000 --- a/flow/SignalSafeUnwind.h +++ /dev/null @@ -1,30 +0,0 @@ -/* - * SignalSafeUnwind.h - * - * This source file is part of the FoundationDB open source project - * - * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef FLOW_SIGNAL_SAFE_UNWIND -#define FLOW_SIGNAL_SAFE_UNWIND -#pragma once - -#include "flow/Platform.h" - -// This can be used by tests to measure the number of calls to dl_iterate_phdr intercepted -extern int64_t dl_iterate_phdr_calls; - -#endif \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fc7b0e2ec7..e572582650 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -94,7 +94,7 @@ if(WITH_PYTHON) else() add_fdb_test(TEST_FILES SimpleExternalTest.txt IGNORE) endif() - add_fdb_test(TEST_FILES SlowTask.txt IGNORE) + add_fdb_test(TEST_FILES noSim/SlowTask.txt IGNORE) add_fdb_test(TEST_FILES SpecificUnitTest.txt IGNORE) add_fdb_test(TEST_FILES StorageMetricsSampleTests.txt IGNORE) add_fdb_test(TEST_FILES WorkerTests.txt IGNORE) diff --git a/tests/SlowTask.txt b/tests/noSim/SlowTask.txt similarity index 82% rename from tests/SlowTask.txt rename to tests/noSim/SlowTask.txt index 1782b29561..2289155d23 100644 --- a/tests/SlowTask.txt +++ b/tests/noSim/SlowTask.txt @@ -1,2 +1,3 @@ +useDB=false testTitle=Slow Task Stress testName=SlowTaskWorkload