diff --git a/src/profiledata.h b/src/profiledata.h index a4f0446..ffe4572 100644 --- a/src/profiledata.h +++ b/src/profiledata.h @@ -143,6 +143,8 @@ class ProfileData { void GetCurrentState(State* state) const; private: + friend class CpuProfiler; + static const int kAssociativity = 4; // For hashtable static const int kBuckets = 1 << 10; // For hashtable static const int kBufferLength = 1 << 18; // For eviction buffer diff --git a/src/profiler.cc b/src/profiler.cc index 2468067..4686814 100644 --- a/src/profiler.cc +++ b/src/profiler.cc @@ -68,12 +68,6 @@ typedef int ucontext_t; // just to quiet the compiler, mostly using std::string; -DEFINE_bool(cpu_profiler_unittest, - EnvToBool("PERFTOOLS_UNITTEST", true), - "Determines whether or not we are running under the \ - control of a unit test. This allows us to include or \ - exclude certain behaviours."); - // Collects up all profile data. This is a singleton, which is // initialized by a constructor at startup. If no cpu profiler // signal is specified then the profiler lifecycle is either @@ -176,22 +170,13 @@ CpuProfiler CpuProfiler::instance_; // Initialize profiling: activated if getenv("CPUPROFILE") exists. CpuProfiler::CpuProfiler() : prof_handler_token_(NULL) { - // TODO(cgd) Move this code *out* of the CpuProfile constructor into a - // separate object responsible for initialization. With ProfileHandler there - // is no need to limit the number of profilers. if (getenv("CPUPROFILE") == NULL) { - if (!FLAGS_cpu_profiler_unittest) { - RAW_LOG(WARNING, "CPU profiler linked but no valid CPUPROFILE environment variable found\n"); - } return; } // We don't enable profiling if setuid -- it's a security risk #ifdef HAVE_GETEUID if (getuid() != geteuid()) { - if (!FLAGS_cpu_profiler_unittest) { - RAW_LOG(WARNING, "Cannot perform CPU profiling when running with setuid\n"); - } return; } #endif @@ -212,11 +197,8 @@ CpuProfiler::CpuProfiler() } else { char fname[PATH_MAX]; if (!GetUniquePathFromEnv("CPUPROFILE", fname)) { - if (!FLAGS_cpu_profiler_unittest) { - RAW_LOG(WARNING, "CPU profiler linked but no valid CPUPROFILE environment variable found\n"); - } return; - } + } if (!Start(fname, NULL)) { RAW_LOG(FATAL, "Can't turn on cpu profiling for '%s': %s\n", @@ -307,9 +289,23 @@ void CpuProfiler::GetCurrentState(ProfilerState* state) { state->enabled = collector_state.enabled; state->start_time = static_cast(collector_state.start_time); state->samples_gathered = collector_state.samples_gathered; - int buf_size = sizeof(state->profile_name); - strncpy(state->profile_name, collector_state.profile_name, buf_size); - state->profile_name[buf_size-1] = '\0'; + + constexpr int kBufSize = sizeof(state->profile_name); + std::string_view profile_name{collector_state.profile_name}; + memcpy(state->profile_name, profile_name.data(), std::min(kBufSize, profile_name.size() + 1)); + state->profile_name[kBufSize - 1] = '\0'; + + // Note, this is "secret" and version-specific API we do for + // profiler_unittest. It is explicitly not part of any API/ABI + // stability guarantees. + // + // If there is space in profile_name left for the pointer, then we + // append address of samples_gathered. The test uses this "ticks + // count" as a form of clock to know how long it runs. + if (profile_name.size() + 1 + sizeof(void*) <= kBufSize) { + void* ptr = &collector_.count_; + memcpy(state->profile_name + profile_name.size() + 1, &ptr, sizeof(ptr)); + } } void CpuProfiler::EnableHandler() { diff --git a/src/tests/profiler_unittest.cc b/src/tests/profiler_unittest.cc index 75de13b..bf2c5b6 100644 --- a/src/tests/profiler_unittest.cc +++ b/src/tests/profiler_unittest.cc @@ -38,10 +38,12 @@ #include "config_for_unittests.h" #include #include +#include + #ifdef HAVE_UNISTD_H #include // for fork() -#endif #include // for wait() +#endif #include #include @@ -49,23 +51,43 @@ #include "gperftools/profiler.h" #include "tests/testutil.h" -static std::atomic result; static int g_iters; // argv[1] +// g_ticks_count points to internal profiler's tick count that +// increments each profiling tick. Makes it possible for this test +// loops to run long enough to get enough ticks. +static int volatile *g_ticks_count = ([] () { + ProfilerState state; + memset(&state, 0, sizeof(state)); + ProfilerGetCurrentState(&state); + size_t sz = strlen(state.profile_name); + if (sz + 1 + sizeof(g_ticks_count) > sizeof(state.profile_name)) { + fprintf(stderr, "too long profile_name?: %zu (%s)\n", sz, state.profile_name); + abort(); + } + int volatile* ptr; + memcpy(&ptr, state.profile_name + sz + 1, sizeof(ptr)); + return ptr; +})(); + std::mutex mutex; static void test_other_thread() { #ifndef NO_THREADS ProfilerRegisterThread(); - int i, m; + int result = 0; char b[128]; + // Get at least 30 ticks + int limit = *g_ticks_count + 30; + std::lock_guard ml(mutex); - for (m = 0; m < 1000000; ++m) { // run millions of times - for (i = 0; i < g_iters; ++i ) { - result ^= i; + + while (*g_ticks_count < limit) { + for (int i = 0; i < g_iters * 10; ++i ) { + *const_cast(&result) ^= i; } - snprintf(b, sizeof(b), "other: %d", result.load()); // get some libc action + snprintf(b, sizeof(b), "other: %d", result); // get some libc action (void)noopt(b); // 'consume' b. Ensure that smart compiler doesn't // remove snprintf call } @@ -73,18 +95,24 @@ static void test_other_thread() { } static void test_main_thread() { - int i, m; + int result = 0; char b[128]; + // Get at least 30 ticks + int limit = *g_ticks_count + 30; + std::lock_guard ml(mutex); - for (m = 0; m < 1000000; ++m) { // run millions of times - for (i = 0; i < g_iters; ++i ) { - result ^= i; + + while (*g_ticks_count < limit) { + for (int i = 0; i < g_iters * 10; ++i ) { + *const_cast(&result) ^= i; } - snprintf(b, sizeof(b), "same: %d", result.load()); // get some libc action + snprintf(b, sizeof(b), "same: %d", result); // get some libc action (void)noopt(b); // 'consume' b } } + + int main(int argc, char** argv) { if ( argc <= 1 ) { fprintf(stderr, "USAGE: %s [num_threads] [filename]\n", argv[0]); diff --git a/src/tests/profiler_unittest.sh b/src/tests/profiler_unittest.sh index 2c8afe3..055fe7a 100755 --- a/src/tests/profiler_unittest.sh +++ b/src/tests/profiler_unittest.sh @@ -2,11 +2,11 @@ # Copyright (c) 2005, Google Inc. # All rights reserved. -# +# # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are # met: -# +# # * Redistributions of source code must retain the above copyright # notice, this list of conditions and the following disclaimer. # * Redistributions in binary form must reproduce the above @@ -16,7 +16,7 @@ # * Neither the name of Google Inc. nor the names of its # contributors may be used to endorse or promote products derived from # this software without specific prior written permission. -# +# # THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS # "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT # LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR @@ -53,15 +53,12 @@ if [ "x$1" = "x-h" -o "x$1" = "x--help" ]; then exit 1 fi -TMPDIR=/tmp/profile_info +# note, this also creates temp directory +TMPDIR=`mktemp -d` UNITTEST_DIR=${1:-$BINDIR} PPROF=${2:-$PPROF_PATH} -# We test the sliding-window functionality of the cpu-profile reader -# by using a small stride, forcing lots of reads. -PPROF_FLAGS="--test_stride=128" - PROFILER1="$UNITTEST_DIR/profiler1_unittest" PROFILER2="$UNITTEST_DIR/profiler2_unittest" PROFILER3="$UNITTEST_DIR/profiler3_unittest" @@ -85,17 +82,6 @@ PROFILER4_REALNAME=`Realname "$PROFILER4"` # It's meaningful to the profiler, so make sure we know its state unset CPUPROFILE -# Some output/logging in the profiler can cause issues when running the unit -# tests. For example, logging a warning when the profiler is detected as being -# present but no CPUPROFILE is specified in the environment. Especially when -# we are checking for a silent run or specific timing constraints are being -# checked. So set the env variable signifying that we are running in a unit -# test environment. -PERFTOOLS_UNITTEST=1 - -rm -rf "$TMPDIR" -mkdir "$TMPDIR" || exit 2 - num_failures=0 RegisterFailure() { @@ -118,8 +104,8 @@ VerifySimilar() { # We are careful not to put exec1 and exec2 in quotes, because if # they are the empty string, it means we want to use the 1-arg # version of pprof. - mthread1=`"$PPROF" $PPROF_FLAGS $exec1 "$prof1" | grep test_main_thread | awk '{print $1}'` - mthread2=`"$PPROF" $PPROF_FLAGS $exec2 "$prof2" | grep test_main_thread | awk '{print $1}'` + mthread1=`"$PPROF" $exec1 "$prof1" | grep test_main_thread | awk '{print $1}'` + mthread2=`"$PPROF" $exec2 "$prof2" | grep test_main_thread | awk '{print $1}'` mthread1_plus=`expr $mthread1 + 5` mthread2_plus=`expr $mthread2 + 5` if [ -z "$mthread1" ] || [ -z "$mthread2" ] || \ @@ -151,8 +137,8 @@ VerifyAcrossThreads() { # We are careful not to put exec1 in quotes, because if it is the # empty string, it means we want to use the 1-arg version of pprof. - mthread=`$PPROF $PPROF_FLAGS $exec1 "$prof1" | grep test_main_thread | awk '{print $1}'` - othread=`$PPROF $PPROF_FLAGS $exec1 "$prof1" | grep test_other_thread | awk '{print $1}'` + mthread=`$PPROF $exec1 "$prof1" | grep test_main_thread | awk '{print $1}'` + othread=`$PPROF $exec1 "$prof1" | grep test_other_thread | awk '{print $1}'` if [ -z "$mthread" ] || [ -z "$othread" ] || \ [ "$mthread" -le 0 -o "$othread" -le 0 ] # || [ `expr $mthread \* $mult \* 3` -gt `expr $othread \* 10` -o \ @@ -166,13 +152,6 @@ VerifyAcrossThreads() { fi } -echo -echo ">>> WARNING <<<" -echo "This test looks at timing information to determine correctness." -echo "If your system is loaded, the test may spuriously fail." -echo "If the test does fail with an 'Actual times' error, try running again." -echo - # profiler1 is a non-threaded version "$PROFILER1" 50 1 "$TMPDIR/p1" || RegisterFailure "$PROFILER1" 100 1 "$TMPDIR/p2" || RegisterFailure