Merge pull request #2 from RedisBloom/origin.fix

Added perf record/report automation to makefile
RedisBloom · Nov 15, 2020 · ca8741c · ca8741c
2 parents 3ccf6d4 + 29d7635
commit ca8741c
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 34 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,5 +1,5 @@
 ## cmake flags
-CMAKE_MINIMUM_REQUIRED(VERSION 3.0.0)
+cmake_minimum_required (VERSION 3.0)
 project(tdigest)
 
 # CMake modules should be included in ${CMAKE_SOURCE_DIR}/cmake
@@ -11,6 +11,7 @@ option(BUILD_STATIC "Build static library" ON)
 option(BUILD_BENCHMARK "Build benchmark" ON)
 option(BUILD_TESTS "Build tests" ON)
 OPTION(ENABLE_CODECOVERAGE "Enable code coverage testing support" OFF)
+OPTION(ENABLE_PROFILE "Enable code profiling support" OFF)
 option(BUILD_EXAMPLES "Build examples" ON)
 
 # --- Build properties ---
@@ -32,16 +33,22 @@ if(ENABLE_CODECOVERAGE)
       set(CMAKE_BUILD_TYPE "Debug" CACHE
               STRING "Choose the type of build." FORCE)
 ENDIF()
+
 # Generate position-independent code (-fPIC on UNIX)
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
-# For adding specific Release flags
 set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O3")
 set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")
 
-# Add -fno-omit-frame-pointer to avoid seeing incomplete stack traces
-set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -g -ggdb -fno-omit-frame-pointer")
-set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g -ggdb -fno-omit-frame-pointer")
+if(ENABLE_PROFILE)
+  message(STATUS "Enabling profile flags.")
+  string (REPLACE "-DNDEBUG" "" CMAKE_C_FLAGS_RELEASE ${CMAKE_C_FLAGS_RELEASE})
+  string (REPLACE "-O3" "" CMAKE_C_FLAGS_RELEASE ${CMAKE_C_FLAGS_RELEASE})
+  string (REPLACE "-O3" "" CMAKE_CXX_FLAGS_RELEASE ${CMAKE_CXX_FLAGS_RELEASE})
+  string (REPLACE "-DNDEBUG" "" CMAKE_CXX_FLAGS_RELEASE ${CMAKE_CXX_FLAGS_RELEASE})
+  set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O2 -g -ggdb -fno-omit-frame-pointer")
+  set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O2  -g -ggdb -fno-omit-frame-pointer")
+endif(ENABLE_PROFILE)
 
 # --- System Libraries ---
 include(GNUInstallDirs)
@@ -66,9 +73,3 @@ endif(BUILD_TESTS)
 if(BUILD_EXAMPLES)
   add_subdirectory("${CMAKE_CURRENT_SOURCE_DIR}/examples")
 endif(BUILD_EXAMPLES)
-
-macro (add_compiler_flags)
-    string (REPLACE ";" " " _FLAGS "${ARGV}")
-    set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${_FLAGS}")
-    set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${_FLAGS}")
-endmacro ()
diff --git a/Makefile b/Makefile
@@ -9,11 +9,11 @@ INFER_DOCKER?=redisbench/infer-linux64:1.0.0
 ROOT=$(shell pwd)
 SRCDIR := $(ROOT)/src
 
+
 ifndef CMAKE_LIBRARY_SHARED_OPTIONS
 	CMAKE_LIBRARY_SHARED_OPTIONS=\
 		-DBUILD_SHARED=ON \
 		-DBUILD_STATIC=OFF \
-		-DENABLE_FRAME_POINTER=ON \
 		-DENABLE_CODECOVERAGE=OFF \
 		-DBUILD_TESTS=OFF \
 		-DBUILD_BENCHMARK=OFF \
@@ -24,41 +24,46 @@ ifndef CMAKE_LIBRARY_STATIC_OPTIONS
 	CMAKE_LIBRARY_STATIC_OPTIONS=\
 		-DBUILD_SHARED=OFF \
 		-DBUILD_STATIC=ON \
-		-DENABLE_FRAME_POINTER=ON \
 		-DENABLE_CODECOVERAGE=OFF \
 		-DBUILD_TESTS=OFF \
-		-DBUILD_BENCHMARK=OFF \
 		-DBUILD_EXAMPLES=OFF
 endif
 
 ifndef CMAKE_LIBRARY_OPTIONS
 	CMAKE_LIBRARY_OPTIONS=\
 		-DBUILD_SHARED=ON \
 		-DBUILD_STATIC=ON \
-		-DENABLE_FRAME_POINTER=ON \
 		-DENABLE_CODECOVERAGE=OFF \
 		-DBUILD_TESTS=OFF \
-		-DBUILD_BENCHMARK=OFF \
 		-DBUILD_EXAMPLES=OFF
 endif
 
 ifndef CMAKE_FULL_OPTIONS
 	CMAKE_FULL_OPTIONS=\
 		-DBUILD_SHARED=ON \
 		-DBUILD_STATIC=ON \
-		-DENABLE_FRAME_POINTER=ON \
-		-DENABLE_CODECOVERAGE=OFF \
 		-DBUILD_TESTS=ON \
 		-DBUILD_BENCHMARK=ON \
 		-DBUILD_EXAMPLES=ON
 endif
 
 
+ifndef CMAKE_PROFILE_OPTIONS
+	CMAKE_PROFILE_OPTIONS=\
+		-DBUILD_SHARED=ON \
+		-DBUILD_STATIC=OFF \
+		-DENABLE_CODECOVERAGE=OFF \
+		-DBUILD_TESTS=ON \
+		-DBUILD_BENCHMARK=ON \
+		-DBUILD_EXAMPLES=OFF \
+		-DENABLE_PROFILE=ON
+endif
+
+
 ifndef CMAKE_TEST_OPTIONS
 	CMAKE_TEST_OPTIONS=\
 		-DBUILD_SHARED=ON \
 		-DBUILD_STATIC=ON \
-		-DENABLE_FRAME_POINTER=ON \
 		-DBUILD_TESTS=ON \
 		-DENABLE_CODECOVERAGE=ON \
 		-DBUILD_BENCHMARK=OFF \
@@ -103,7 +108,7 @@ lint:
 
 # build all
 full:
-	( mkdir -p build; cd build ; cmake $(CMAKE_FULL_OPTIONS) .. ; $(MAKE) VERBOSE=1 )
+	( mkdir -p build; cd build ; cmake $(CMAKE_FULL_OPTIONS) .. ; $(MAKE) )
 
 # static-analysis-docker:
 # 	$(MAKE) clean
@@ -115,16 +120,20 @@ distclean:
 	rm -rf build/* 
 
 bench: clean
-	CFLAGS="-g -fno-omit-frame-pointer " CXXFLAGS="-g -fno-omit-frame-pointer " $(MAKE)
+	( mkdir -p build; cd build ; cmake $(CMAKE_PROFILE_OPTIONS) .. ; $(MAKE) VERBOSE=1 )
 	$(SHOW) build/tests/histogram_benchmark --benchmark_min_time=10
 
 perf-stat-bench:
-	CFLAGS="-g -fno-omit-frame-pointer " CXXFLAGS="-g -fno-omit-frame-pointer " $(MAKE)
+	( mkdir -p build; cd build ; cmake $(CMAKE_PROFILE_OPTIONS) .. ; $(MAKE) VERBOSE=1 )
 	$(SHOW) perf stat build/tests/histogram_benchmark --benchmark_min_time=10
 
-perf-record-bench:
-	CFLAGS="-g -fno-omit-frame-pointer " CXXFLAGS="-g -fno-omit-frame-pointer " $(MAKE)
-	$(SHOW) perf record -g -o perf.data.td_add build/tests/histogram_benchmark --benchmark_min_time=10
+perf-record-bench: clean
+	( mkdir -p build; cd build ; cmake $(CMAKE_PROFILE_OPTIONS) .. ; $(MAKE) VERBOSE=1 )
+	$(SHOW) perf record -g -o perf.data.td_add \
+		build/tests/histogram_benchmark
+
+perf-report-bench:
+	$(SHOW) perf report -g "graph,0.5,caller" -i perf.data.td_add
 
-perf-report-bench: 
-	$(SHOW) perf report -g 'graph,0.5,caller' -i perf.data.td_add
+perf-report-bench-pprof:
+	go tool pprof -web perf.data.td_add
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -2,10 +2,10 @@ FILE(GLOB c_files "*.c")
 FILE(GLOB header_files "*.h")
 
 if (BUILD_SHARED)
-    add_library(tdigest SHARED ${c_files} ${header_files}) 
+    add_library(tdigest SHARED ${c_files} ${header_files})
     target_link_libraries(tdigest m)
     target_include_directories(tdigest SYSTEM PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
-    install(TARGETS tdigest DESTINATION lib${LIB_SUFFIX}) 
+    install(TARGETS tdigest DESTINATION lib${LIB_SUFFIX})
 endif(BUILD_SHARED)
 
 if (BUILD_STATIC) 

diff --git a/src/tdigest.c b/src/tdigest.c
@@ -184,13 +184,15 @@ void td_add(td_histogram_t *h, double mean, double count) {
 static int compare_nodes(const void *v1, const void *v2) {
     node_t *n1 = (node_t *)(v1);
     node_t *n2 = (node_t *)(v2);
-    if (n1->mean < n2->mean) {
+    const double n1m = n1->mean;
+    const double n2m = n2->mean;
+    if (n1m < n2m) {
         return -1;
-    } else if (n1->mean > n2->mean) {
+    }
+    if (n1m > n2m) {
         return 1;
-    } else {
-        return 0;
     }
+    return 0;
 }
 
 void td_compress(td_histogram_t *h) {

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -1,11 +1,13 @@
 
 if (BUILD_BENCHMARK)
     if (UNIX)
+        set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O2 -g -ggdb -fno-omit-frame-pointer")
+        set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O2  -g -ggdb -fno-omit-frame-pointer")
         set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "Suppressing benchmark's tests" FORCE)
         add_subdirectory(vendor/google/benchmark)
         include_directories(vendor/google/benchmark/include)
         add_executable(histogram_benchmark histogram_benchmark.cpp)
-        target_link_libraries(histogram_benchmark tdigest benchmark)
+        target_link_libraries(histogram_benchmark tdigest benchmark::benchmark)
     else()
         message(WARNING
               "google.benchmark - microbenchmarks disabled on WIN32 platforms")