From 631b9f512e0efc6630531a873ee0f255725cf7a6 Mon Sep 17 00:00:00 2001
From: danieldoglas <dsilva@expensify.com>
Date: Tue, 8 Oct 2024 14:16:15 +0000
Subject: [PATCH] addressing PR comments

---
 libstuff/ResourceMonitorThread.cpp | 22 +++++++++++-----------
 libstuff/ResourceMonitorThread.h   |  8 ++++----
 libstuff/libstuff.cpp              |  1 +
 libstuff/libstuff.h                |  3 ++-
 4 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/libstuff/ResourceMonitorThread.cpp b/libstuff/ResourceMonitorThread.cpp
index 59f9ba04b..08b0cecbb 100644
--- a/libstuff/ResourceMonitorThread.cpp
+++ b/libstuff/ResourceMonitorThread.cpp
@@ -1,26 +1,26 @@
 #include "ResourceMonitorThread.h"
 #include "libstuff/libstuff.h"
-#include "format"
+#include <format>
 #include <cmath>
 
-thread_local uint64_t ResourceMonitorThread::startTime;
-thread_local double ResourceMonitorThread::cpuStartTime;
+uint64_t ResourceMonitorThread::threadStartTime;
+double ResourceMonitorThread::cpuStartTime;
 
-void ResourceMonitorThread::before(){
-    startTime = STimeNow();
+void ResourceMonitorThread::before() {
+    threadStartTime = STimeNow();
     cpuStartTime = SGetCPUUserTime();
 }
 
-void ResourceMonitorThread::after(){
-    const uint64_t threadEndTime = STimeNow() - startTime;
-    const double CPUUserTime = SGetCPUUserTime() - cpuStartTime;
+void ResourceMonitorThread::after() {
+    const uint64_t threadUserTime = STimeNow() - threadStartTime;
+    const double cpuUserTime = SGetCPUUserTime() - cpuStartTime;
 
     // This shouldn't happen since the time to start/finish a thread should take more than a microsecond, but to be 
     // sure we're not dividing by 0 and causing crashes, let's add an if here and return if threadEndTime is 0.
-    if (threadEndTime == 0) {
+    if (threadUserTime == 0) {
         return;
     }
-    const double cpuUserPercentage = round((CPUUserTime / static_cast<double>(threadEndTime)) * 100 * 1000) / 1000;
+    const double cpuUserPercentage = round((cpuUserTime / static_cast<double>(threadUserTime)) * 100 * 1000) / 1000;
     const pid_t tid = syscall(SYS_gettid);
-    SINFO(format("Thread finished. pID: '{}', CPUTime: '{}µs', CPUPercentage: '{}%'", tid, CPUUserTime, cpuUserPercentage, startTime, cpuStartTime));
+    SINFO(format("Thread finished. pID: '{}', CPUTime: '{}µs', CPUPercentage: '{}%'", tid, cpuUserTime, cpuUserPercentage));
 }
diff --git a/libstuff/ResourceMonitorThread.h b/libstuff/ResourceMonitorThread.h
index cead977f6..54729f504 100644
--- a/libstuff/ResourceMonitorThread.h
+++ b/libstuff/ResourceMonitorThread.h
@@ -6,7 +6,7 @@ using namespace std;
 // This class is a wrapper around the default thread. We use it to collect the thread CPU usage. That allows us
 // to investigate if we have any threads using more resources than it should, which can cause CPU usage peaks in
 // the cluster.
-class ResourceMonitorThread: public thread
+class ResourceMonitorThread : public thread
 {
 public:
     // When calling this constructor, if you're passing a class member function as the `f` parameter and that
@@ -16,8 +16,8 @@ class ResourceMonitorThread: public thread
     ResourceMonitorThread(F&& f, Args&&... args):
       thread(ResourceMonitorThread::wrapper<F&&, Args&&...>, forward<F&&>(f), forward<Args&&>(args)...){};
 private:
-    static thread_local uint64_t startTime;
-    static thread_local double cpuStartTime;
+    static uint64_t threadStartTime;
+    static double cpuStartTime;
 
     static void before();
     static void after();
@@ -25,7 +25,7 @@ class ResourceMonitorThread: public thread
     template<typename F, typename... Args>
     static void wrapper(F&& f, Args&&... args) {
         before();
-        std::invoke(std::forward<F>(f), std::forward<Args>(args)...);
+        invoke(forward<F>(f), forward<Args>(args)...);
         after();
     }
 };
\ No newline at end of file
diff --git a/libstuff/libstuff.cpp b/libstuff/libstuff.cpp
index 78ab7bcf5..5dd682d10 100644
--- a/libstuff/libstuff.cpp
+++ b/libstuff/libstuff.cpp
@@ -3206,6 +3206,7 @@ SString& SString::operator=(const bool from) {
 double SGetCPUUserTime() {
     struct rusage usage;
     getrusage(RUSAGE_THREAD, &usage);
+
     // Returns the current threads CPU user time in microseconds
     return static_cast<double>(usage.ru_utime.tv_sec) * 1e6 + static_cast<double>(usage.ru_utime.tv_usec);
 }
diff --git a/libstuff/libstuff.h b/libstuff/libstuff.h
index fb4a55e66..13a158179 100644
--- a/libstuff/libstuff.h
+++ b/libstuff/libstuff.h
@@ -629,6 +629,7 @@ string SGUnzip(const string& content);
 // Command-line helpers
 STable SParseCommandLine(int argc, char* argv[]);
 
-// Returns the current CPU usage inside the current process
+// Returns the CPU usage inside the current thread
 double SGetCPUUserTime();
+
 #endif	// LIBSTUFF_H