Back out D53198236 and D53146503 (facebookincubator#8661)

Summary:
Pull Request resolved: facebookincubator#8661

Back out "[velox] Add TraceHistory" and "[velox] Add TraceContext to various system calls that can block". This addresses https://fb.workplace.com/groups/velox.users/posts/25769555209333084

Reviewed By: spershin

Differential Revision: D53365055

fbshipit-source-id: 55c9738305a1d693cb5c975f0eed030f650a097f

velox/common/caching/CMakeLists.txt

@@ -28,7 +28,6 @@ target_link_libraries(
          velox_exception
          velox_file
          velox_memory
-         velox_process
          Folly::folly
          fmt::fmt
          gflags::gflags

velox/common/caching/SsdFile.cpp

@@ -15,14 +15,12 @@
  */
 
 #include "velox/common/caching/SsdFile.h"
-
 #include <folly/Executor.h>
 #include <folly/portability/SysUio.h>
 #include "velox/common/base/AsyncSource.h"
 #include "velox/common/base/SuccinctPrinter.h"
 #include "velox/common/caching/FileIds.h"
 #include "velox/common/caching/SsdCache.h"
-#include "velox/common/process/TraceContext.h"
 
 #include <fcntl.h>
 #ifdef linux
@@ -130,7 +128,6 @@ SsdFile::SsdFile(
       shardId_(shardId),
       checkpointIntervalBytes_(checkpointIntervalBytes),
       executor_(executor) {
-  process::TraceContext trace("SsdFile::SsdFile");
   int32_t oDirect = 0;
 #ifdef linux
   oDirect = FLAGS_ssd_odirect ? O_DIRECT : 0;
@@ -269,7 +266,6 @@ CoalesceIoStats SsdFile::load(
 void SsdFile::read(
     uint64_t offset,
     const std::vector<folly::Range<char*>>& buffers) {
-  process::TraceContext trace("SsdFile::read");
   readFile_->preadv(offset, buffers);
 }
 
@@ -311,7 +307,6 @@ std::optional<std::pair<uint64_t, int32_t>> SsdFile::getSpace(
 }
 
 bool SsdFile::growOrEvictLocked() {
-  process::TraceContext trace("SsdFile::growOrEvictLocked");
   if (numRegions_ < maxRegions_) {
     const auto newSize = (numRegions_ + 1) * kRegionSize;
     const auto rc = ::ftruncate(fd_, newSize);
@@ -365,7 +360,6 @@ void SsdFile::clearRegionEntriesLocked(const std::vector<int32_t>& regions) {
 }
 
 void SsdFile::write(std::vector<CachePin>& pins) {
-  process::TraceContext trace("SsdFile::write");
   // Sorts the pins by their file/offset. In this way what is adjacent in
   // storage is likely adjacent on SSD.
   std::sort(pins.begin(), pins.end());
@@ -450,7 +444,6 @@ int32_t indexOfFirstMismatch(char* x, char* y, int n) {
 } // namespace
 
 void SsdFile::verifyWrite(AsyncDataCacheEntry& entry, SsdRun ssdRun) {
-  process::TraceContext trace("SsdFile::verifyWrite");
   auto testData = std::make_unique<char[]>(entry.size());
   const auto rc = ::pread(fd_, testData.get(), entry.size(), ssdRun.offset());
   VELOX_CHECK_EQ(rc, entry.size());
@@ -519,7 +512,6 @@ void SsdFile::clear() {
 }
 
 void SsdFile::deleteFile() {
-  process::TraceContext trace("SsdFile::deleteFile");
   if (fd_) {
     close(fd_);
     fd_ = 0;
@@ -659,7 +651,6 @@ inline const char* asChar(const T* ptr) {
 } // namespace
 
 void SsdFile::checkpoint(bool force) {
-  process::TraceContext trace("SsdFile::checkpoint");
   std::lock_guard<std::shared_mutex> l(mutex_);
   if (!force && (bytesAfterCheckpoint_ < checkpointIntervalBytes_)) {
     return;

velox/common/process/CMakeLists.txt

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 add_library(velox_process ProcessBase.cpp StackTrace.cpp ThreadDebugInfo.cpp
-                          TraceContext.cpp TraceHistory.cpp)
+                          TraceContext.cpp)
 
 target_link_libraries(
   velox_process

velox/common/process/TraceContext.cpp

@@ -16,34 +16,23 @@
 
 #include "velox/common/process/TraceContext.h"
 
-#include "velox/common/process/ThreadLocalRegistry.h"
-#include "velox/common/process/TraceHistory.h"
-
 #include <sstream>
 
 namespace facebook::velox::process {
 
 namespace {
-
-// We use thread local instead lock here since the critical path is on write
-// side.
-using Registry = ThreadLocalRegistry<folly::F14FastMap<std::string, TraceData>>;
-auto registry = std::make_shared<Registry>();
-thread_local Registry::Reference threadLocalTraceData(registry);
-
+folly::Synchronized<std::unordered_map<std::string, TraceData>>& traceMap() {
+  static folly::Synchronized<std::unordered_map<std::string, TraceData>>
+      staticTraceMap;
+  return staticTraceMap;
+}
 } // namespace
 
 TraceContext::TraceContext(std::string label, bool isTemporary)
     : label_(std::move(label)),
       enterTime_(std::chrono::steady_clock::now()),
       isTemporary_(isTemporary) {
-  TraceHistory::push([&](auto& entry) {
-    entry.time = enterTime_;
-    entry.file = __FILE__;
-    entry.line = __LINE__;
-    snprintf(entry.label, entry.kLabelCapacity, "%s", label_.c_str());
-  });
-  threadLocalTraceData.withValue([&](auto& counts) {
+  traceMap().withWLock([&](auto& counts) {
     auto& data = counts[label_];
     ++data.numThreads;
     if (data.numThreads == 1) {
@@ -54,59 +43,45 @@ TraceContext::TraceContext(std::string label, bool isTemporary)
 }
 
 TraceContext::~TraceContext() {
-  threadLocalTraceData.withValue([&](auto& counts) {
-    auto it = counts.find(label_);
-    auto& data = it->second;
-    if (--data.numThreads == 0 && isTemporary_) {
-      counts.erase(it);
-      return;
-    }
+  traceMap().withWLock([&](auto& counts) {
+    auto& data = counts[label_];
+    --data.numThreads;
     auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(
                   std::chrono::steady_clock::now() - enterTime_)
                   .count();
     data.totalMs += ms;
     data.maxMs = std::max<uint64_t>(data.maxMs, ms);
+    if (!data.numThreads && isTemporary_) {
+      counts.erase(label_);
+    }
   });
 }
 
 // static
 std::string TraceContext::statusLine() {
   std::stringstream out;
   auto now = std::chrono::steady_clock::now();
-  auto counts = status();
-  for (auto& pair : counts) {
-    if (pair.second.numThreads) {
-      auto continued = std::chrono::duration_cast<std::chrono::milliseconds>(
-                           now - pair.second.startTime)
-                           .count();
+  traceMap().withRLock([&](auto& counts) {
+    for (auto& pair : counts) {
+      if (pair.second.numThreads) {
+        auto continued = std::chrono::duration_cast<std::chrono::milliseconds>(
+                             now - pair.second.startTime)
+                             .count();
 
-      out << pair.first << "=" << pair.second.numThreads << " entered "
-          << pair.second.numEnters << " avg ms "
-          << (pair.second.totalMs / pair.second.numEnters) << " max ms "
-          << pair.second.maxMs << " continuous for " << continued << std::endl;
+        out << pair.first << "=" << pair.second.numThreads << " entered "
+            << pair.second.numEnters << " avg ms "
+            << (pair.second.totalMs / pair.second.numEnters) << " max ms "
+            << pair.second.maxMs << " continuous for " << continued
+            << std::endl;
+      }
     }
-  }
+  });
   return out.str();
 }
 
 // static
-folly::F14FastMap<std::string, TraceData> TraceContext::status() {
-  folly::F14FastMap<std::string, TraceData> total;
-  registry->forAllValues([&](auto& counts) {
-    for (auto& [k, v] : counts) {
-      auto& sofar = total[k];
-      if (sofar.numEnters == 0) {
-        sofar.startTime = v.startTime;
-      } else if (v.numEnters > 0) {
-        sofar.startTime = std::min(sofar.startTime, v.startTime);
-      }
-      sofar.numThreads += v.numThreads;
-      sofar.numEnters += v.numEnters;
-      sofar.totalMs += v.totalMs;
-      sofar.maxMs = std::max(sofar.maxMs, v.maxMs);
-    }
-  });
-  return total;
+std::unordered_map<std::string, TraceData> TraceContext::status() {
+  return traceMap().withRLock([&](auto& map) { return map; });
 }
 
 } // namespace facebook::velox::process

velox/common/process/TraceContext.h

@@ -20,7 +20,7 @@
 #include <string>
 #include <unordered_map>
 
-#include <folly/container/F14Map.h>
+#include <folly/Synchronized.h>
 
 namespace facebook::velox::process {
 
@@ -63,7 +63,7 @@ class TraceContext {
   static std::string statusLine();
 
   // Returns a copy of the trace status.
-  static folly::F14FastMap<std::string, TraceData> status();
+  static std::unordered_map<std::string, TraceData> status();
 
  private:
   const std::string label_;