Makefile minor cleanup

Summary:
Just couple of small changes:
1. removed signal_test, since it doesn't seem useful and we don't even run it as part of `make check`
2. moved perf_context_test to TESTS instead of PROGRAMS
3. `make release` probably shouldn't compile benchmarks. We currently rely on `make release` building db_bench (via Jenkins), so I left db_bench there.

This is just a minor cleanup. We need to rethink our targets since they are a bit messy right now. We can do this during our tech debt week.

Test Plan: make release

Reviewers: anthony, rven, yhchiang, sdong, meyering

Reviewed By: meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36171

Makefile

@@ -252,7 +252,8 @@ TESTS = \
 	compaction_job_test \
 	thread_list_test \
 	sst_dump_test \
-	compact_files_test
+	compact_files_test \
+	perf_context_test
 
 SUBSET :=  $(shell echo $(TESTS) |sed s/^.*$(ROCKSDBTESTS_START)/$(ROCKSDBTESTS_START)/)
 
@@ -261,10 +262,9 @@ TOOLS = \
 	db_sanity_test \
 	db_stress \
 	ldb \
-	db_repl_stress \
+	db_repl_stress
-	options_test \
 
-PROGRAMS = db_bench signal_test table_reader_bench log_and_apply_bench cache_bench perf_context_test memtablerep_bench $(TOOLS)
+BENCHMARKS = db_bench table_reader_bench log_and_apply_bench cache_bench memtablerep_bench
 
 # The library name is configurable since we are maintaining libraries of both
 # debug/release mode.
@@ -316,18 +316,18 @@ endif  # PLATFORM_SHARED_EXT
 	release tags valgrind_check whitebox_crash_test format static_lib shared_lib all \
 	dbg rocksdbjavastatic rocksdbjava install uninstall analyze
 
-all: $(LIBRARY) $(PROGRAMS) $(TESTS)
+all: $(LIBRARY) $(BENCHMARKS) $(TOOLS) $(TESTS)
 
 static_lib: $(LIBRARY)
 
 shared_lib: $(SHARED)
 
-dbg: $(LIBRARY) $(PROGRAMS) $(TESTS)
+dbg: $(LIBRARY) $(BENCHMARKS) $(TOOLS) $(TESTS)
 
 # creates static library and programs
 release:
 	$(MAKE) clean
-	OPT="-DNDEBUG -O2" $(MAKE) static_lib $(PROGRAMS) -j32
+	OPT="-DNDEBUG -O2" $(MAKE) static_lib $(TOOLS) db_bench -j32
 
 coverage:
 	$(MAKE) clean
@@ -336,7 +336,7 @@ coverage:
 	# Delete intermediate files
 	find . -type f -regex ".*\.\(\(gcda\)\|\(gcno\)\)" -exec rm {} \;
 
-check: $(TESTS) $(PROGRAMS)
+check: all
 	for t in $(TESTS); do echo "***** Running $$t"; ./$$t || exit 1; done
 	python tools/ldb_test.py
 
@@ -365,7 +365,7 @@ asan_crash_test:
 	COMPILE_WITH_ASAN=1 $(MAKE) crash_test
 	$(MAKE) clean
 
-valgrind_check: all $(PROGRAMS) $(TESTS)
+valgrind_check: $(TESTS)
 	mkdir -p $(VALGRIND_DIR)
 	echo TESTS THAT HAVE VALGRIND ERRORS > $(VALGRIND_DIR)/valgrind_failed_tests; \
 	echo TIMES in seconds TAKEN BY TESTS ON VALGRIND > $(VALGRIND_DIR)/valgrind_tests_times; \
@@ -398,7 +398,7 @@ unity: unity.o
 	$(AM_LINK)
 
 clean:
-	rm -f $(PROGRAMS) $(TESTS) $(LIBRARY) $(SHARED) make_config.mk unity.cc
+	rm -f $(BENCHMARKS) $(TOOLS) $(TESTS) $(LIBRARY) $(SHARED) make_config.mk unity.cc
 	rm -f util/build_version.cc
 	rm -rf ios-x86 ios-arm scan_build_report
 	find . -name "*.[oda]" -exec rm {} \;
@@ -443,9 +443,6 @@ db_sanity_test: tools/db_sanity_test.o $(LIBOBJECTS) $(TESTUTIL)
 db_repl_stress: tools/db_repl_stress.o $(LIBOBJECTS) $(TESTUTIL)
 	$(AM_LINK)
 
-signal_test: util/signal_test.o $(LIBOBJECTS)
-	$(AM_LINK)
-
 arena_test: util/arena_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 

db/perf_context_test.cc

@@ -24,6 +24,7 @@ int FLAGS_total_keys = 100;
 int FLAGS_write_buffer_size = 1000000000;
 int FLAGS_max_write_buffer_number = 8;
 int FLAGS_min_write_buffer_number_to_merge = 7;
+bool FLAGS_verbose = false;
 
 // Path to the database on file system
 const std::string kDbName = rocksdb::test::TmpDir() + "/perf_context_test";
@@ -43,9 +44,6 @@ std::shared_ptr<DB> OpenDb(bool read_only = false) {
 #ifndef ROCKSDB_LITE
       options.prefix_extractor.reset(rocksdb::NewFixedPrefixTransform(0));
       options.memtable_factory.reset(NewHashSkipListRepFactory());
-#else
-      fprintf(stderr, "Prefix hash is not supported in lite mode\n");
-      exit(1);
 #endif  // ROCKSDB_LITE
     }
 
@@ -95,8 +93,10 @@ TEST_F(PerfContextTest, SeekIntoDeletion) {
     hist_get_time.Add(elapsed_nanos);
   }
 
+  if (FLAGS_verbose) {
     std::cout << "Get user key comparison: \n" << hist_get.ToString()
               << "Get time: \n" << hist_get_time.ToString();
+  }
 
   {
     HistogramImpl hist_seek_to_first;
@@ -108,6 +108,7 @@ TEST_F(PerfContextTest, SeekIntoDeletion) {
     hist_seek_to_first.Add(perf_context.user_key_comparison_count);
     auto elapsed_nanos = timer.ElapsedNanos();
 
+    if (FLAGS_verbose) {
       std::cout << "SeekToFirst uesr key comparison: \n"
                 << hist_seek_to_first.ToString()
                 << "ikey skipped: " << perf_context.internal_key_skipped_count
@@ -116,6 +117,7 @@ TEST_F(PerfContextTest, SeekIntoDeletion) {
                 << perf_context.internal_delete_skipped_count << "\n"
                 << "elapsed: " << elapsed_nanos << "\n";
     }
+  }
 
   HistogramImpl hist_seek;
   for (int i = 0; i < FLAGS_total_keys; ++i) {
@@ -127,22 +129,29 @@ TEST_F(PerfContextTest, SeekIntoDeletion) {
     iter->Seek(key);
     auto elapsed_nanos = timer.ElapsedNanos();
     hist_seek.Add(perf_context.user_key_comparison_count);
+    if (FLAGS_verbose) {
       std::cout << "seek cmp: " << perf_context.user_key_comparison_count
                 << " ikey skipped " << perf_context.internal_key_skipped_count
-              << " idelete skipped " << perf_context.internal_delete_skipped_count
+                << " idelete skipped "
+                << perf_context.internal_delete_skipped_count
                 << " elapsed: " << elapsed_nanos << "ns\n";
+    }
 
     perf_context.Reset();
     ASSERT_TRUE(iter->Valid());
     StopWatchNano timer2(Env::Default(), true);
     iter->Next();
     auto elapsed_nanos2 = timer2.ElapsedNanos();
+    if (FLAGS_verbose) {
       std::cout << "next cmp: " << perf_context.user_key_comparison_count
                 << "elapsed: " << elapsed_nanos2 << "ns\n";
     }
+  }
 
+  if (FLAGS_verbose) {
     std::cout << "Seek uesr key comparison: \n" << hist_seek.ToString();
   }
+}
 
 TEST_F(PerfContextTest, StopWatchNanoOverhead) {
   // profile the timer cost by itself!
@@ -159,8 +168,10 @@ TEST_F(PerfContextTest, StopWatchNanoOverhead) {
     histogram.Add(timing);
   }
 
+  if (FLAGS_verbose) {
     std::cout << histogram.ToString();
   }
+}
 
 TEST_F(PerfContextTest, StopWatchOverhead) {
   // profile the timer cost by itself!
@@ -180,8 +191,10 @@ TEST_F(PerfContextTest, StopWatchOverhead) {
     prev_timing = timing;
   }
 
+  if (FLAGS_verbose) {
     std::cout << histogram.ToString();
   }
+}
 
 void ProfileQueries(bool enabled_time = false) {
   DestroyDB(kDbName, Options());    // Start this test with a fresh DB
@@ -213,7 +226,9 @@ void ProfileQueries(bool enabled_time = false) {
 
   uint64_t total_db_mutex_nanos = 0;
 
+  if (FLAGS_verbose) {
     std::cout << "Inserting " << FLAGS_total_keys << " key/value pairs\n...\n";
+  }
 
   std::vector<int> keys;
   const int kFlushFlag = -1;
@@ -287,18 +302,19 @@ void ProfileQueries(bool enabled_time = false) {
     hist_mget.Add(perf_context.user_key_comparison_count);
   }
 
+  if (FLAGS_verbose) {
     std::cout << "Put uesr key comparison: \n" << hist_put.ToString()
               << "Get uesr key comparison: \n" << hist_get.ToString()
               << "MultiGet uesr key comparison: \n" << hist_get.ToString();
     std::cout << "Put(): Pre and Post Process Time: \n"
-            << hist_write_pre_post.ToString()
+              << hist_write_pre_post.ToString() << " Writing WAL time: \n"
-            << " Writing WAL time: \n"
               << hist_write_wal_time.ToString() << "\n"
               << " Writing Mem Table time: \n"
               << hist_write_memtable_time.ToString() << "\n"
               << " Total DB mutex nanos: \n" << total_db_mutex_nanos << "\n";
 
-  std::cout << "Get(): Time to get snapshot: \n" << hist_get_snapshot.ToString()
+    std::cout << "Get(): Time to get snapshot: \n"
+              << hist_get_snapshot.ToString()
               << " Time to get value from memtables: \n"
               << hist_get_memtable.ToString() << "\n"
               << " Time to get value from output files: \n"
@@ -316,8 +332,9 @@ void ProfileQueries(bool enabled_time = false) {
               << hist_mget_files.ToString() << "\n"
               << " Number of memtables checked: \n"
               << hist_mget_num_memtable_checked.ToString() << "\n"
-            << " Time to post process: \n" << hist_mget_post_process.ToString()
+              << " Time to post process: \n"
-            << "\n";
+              << hist_mget_post_process.ToString() << "\n";
+  }
 
   if (enabled_time) {
     ASSERT_GT(hist_get.Average(), 0);
@@ -381,6 +398,7 @@ void ProfileQueries(bool enabled_time = false) {
     hist_mget.Add(perf_context.user_key_comparison_count);
   }
 
+  if (FLAGS_verbose) {
     std::cout << "ReadOnly Get uesr key comparison: \n" << hist_get.ToString()
               << "ReadOnly MultiGet uesr key comparison: \n"
               << hist_mget.ToString();
@@ -404,8 +422,9 @@ void ProfileQueries(bool enabled_time = false) {
               << hist_mget_files.ToString() << "\n"
               << " Number of memtables checked: \n"
               << hist_mget_num_memtable_checked.ToString() << "\n"
-            << " Time to post process: \n" << hist_mget_post_process.ToString()
+              << " Time to post process: \n"
-            << "\n";
+              << hist_mget_post_process.ToString() << "\n";
+  }
 
   if (enabled_time) {
     ASSERT_GT(hist_get.Average(), 0);
@@ -454,7 +473,9 @@ TEST_F(PerfContextTest, SeekKeyComparison) {
   WriteOptions write_options;
   ReadOptions read_options;
 
+  if (FLAGS_verbose) {
     std::cout << "Inserting " << FLAGS_total_keys << " key/value pairs\n...\n";
+  }
 
   std::vector<int> keys;
   for (int i = 0; i < FLAGS_total_keys; ++i) {
@@ -484,9 +505,11 @@ TEST_F(PerfContextTest, SeekKeyComparison) {
     hist_time_diff.Add(put_time - perf_context.write_wal_time);
   }
 
-  std::cout << "Put time:\n" << hist_put_time.ToString()
+  if (FLAGS_verbose) {
-            << "WAL time:\n" << hist_wal_time.ToString()
+    std::cout << "Put time:\n" << hist_put_time.ToString() << "WAL time:\n"
-            << "time diff:\n" << hist_time_diff.ToString();
+              << hist_wal_time.ToString() << "time diff:\n"
+              << hist_time_diff.ToString();
+  }
 
   HistogramImpl hist_seek;
   HistogramImpl hist_next;
@@ -510,8 +533,10 @@ TEST_F(PerfContextTest, SeekKeyComparison) {
     hist_next.Add(perf_context.user_key_comparison_count);
   }
 
-  std::cout << "Seek:\n" << hist_seek.ToString()
+  if (FLAGS_verbose) {
-            << "Next:\n" << hist_next.ToString();
+    std::cout << "Seek:\n" << hist_seek.ToString() << "Next:\n"
+              << hist_next.ToString();
+  }
 }
 
 }
@@ -541,9 +566,15 @@ int main(int argc, char** argv) {
       FLAGS_use_set_based_memetable = n;
     }
 
+    if (sscanf(argv[i], "--verbose=%d%c", &n, &junk) == 1 &&
+        (n == 0 || n == 1)) {
+      FLAGS_verbose = n;
+    }
   }
 
+  if (FLAGS_verbose) {
     std::cout << kDbName << "\n";
+  }
 
   return RUN_ALL_TESTS();
 }

src.mk

@@ -216,7 +216,6 @@ TEST_BENCH_SOURCES =                                                    \
   util/options_test.cc                                                  \
   util/event_logger_test.cc                                             \
   util/rate_limiter_test.cc                                             \
-  util/signal_test.cc                                                   \
   util/slice_transform_test.cc                                          \
   util/sst_dump_test.cc                                                 \
   util/testharness.cc                                                   \

util/signal_test.cc

@@ -1,38 +0,0 @@
-//  Copyright (c) 2013, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under the BSD-style license found in the
-//  LICENSE file in the root directory of this source tree. An additional grant
-//  of patent rights can be found in the PATENTS file in the same directory.
-//
-#include "port/stack_trace.h"
-#include <assert.h>
-
-namespace {
-void f0() {
-  char *p = nullptr;
-
-#ifndef __clang_analyzer__
-  // cppcheck-suppress nullPointer
-  *p = 10;  /* SIGSEGV here!! */
-#endif
-}
-
-void f1() {
-  f0();
-}
-
-void f2() {
-  f1();
-}
-
-void f3() {
-  f2();
-}
-}  // namespace
-
-int main() {
-  rocksdb::port::InstallStackTraceHandler();
-
-  f3();
-
-  return 0;
-}