Attend to review comments

elastic · edsavage · Apr 4, 2025 · Apr 4, 2025 · Apr 4, 2025 · Apr 7, 2025
commit 9476edee78422a088b9c94d104680203d1c21bf2
diff --git a/bin/autodetect/Main.cc b/bin/autodetect/Main.cc
@@ -85,8 +85,8 @@ int main(int argc, char** argv) {
         ml::counter_t::E_TSADNumberPrunedItems,
         ml::counter_t::E_TSADAssignmentMemoryBasis,
         ml::counter_t::E_TSADOutputMemoryAllocatorUsage,
-        ml::counter_t::E_TSADResidentSetSize,
-        ml::counter_t::E_TSADMaxResidentSetSize};
+        ml::counter_t::E_TSADSystemMemoryUsage,
+        ml::counter_t::E_TSADMaxSystemMemoryUsage};
 
     ml::core::CProgramCounters::registerProgramCounterTypes(counters);
 
@@ -154,7 +154,7 @@ int main(int argc, char** argv) {
     }
     cancellerThread.stop();
 
-    LOG_DEBUG(<< "Max Resident Set Size: " << ml::core::CProcessStats::maxResidentSetSize());
+    LOG_DEBUG(<< "Resident Set Size: " << ml::core::CProcessStats::residentSetSize());
 
     // Log the program version immediately after reconfiguring the logger.  This
     // must be done from the program, and NOT a shared library, as each program

diff --git a/include/core/CProgramCounters.h b/include/core/CProgramCounters.h
@@ -113,10 +113,10 @@ enum ECounterTypes {
     E_TSADOutputMemoryAllocatorUsage = 30,
 
     //! The resident set size of the process, in bytes.
-    E_TSADResidentSetSize = 31,
+    E_TSADSystemMemoryUsage = 31,
 
     //! The maximum resident set size of the process, in bytes.
-    E_TSADMaxResidentSetSize = 32,
+    E_TSADMaxSystemMemoryUsage = 32,
 
     // Data Frame Outlier Detection
 
@@ -361,9 +361,9 @@ class CORE_EXPORT CProgramCounters {
           "Which option is being used to get model memory for node assignment?"},
          {counter_t::E_TSADOutputMemoryAllocatorUsage, "E_TSADOutputMemoryAllocatorUsage",
           "The amount of memory used to output JSON documents, in bytes."},
-         {counter_t::E_TSADResidentSetSize, "E_TSADResidentSetSize",
+         {counter_t::E_TSADSystemMemoryUsage, "E_TSADResidentSetSize",
           "The resident set size of the process, in bytes"},
-         {counter_t::E_TSADMaxResidentSetSize, "E_TSADMaxResidentSetSize",
+         {counter_t::E_TSADMaxSystemMemoryUsage, "E_TSADMaxResidentSetSize",
           "The maximum resident set size of the process, in bytes"},
          {counter_t::E_DFOEstimatedPeakMemoryUsage, "E_DFOEstimatedPeakMemoryUsage",
           "The upfront estimate of the peak memory outlier detection would use"},

diff --git a/include/model/CResourceMonitor.h b/include/model/CResourceMonitor.h
@@ -182,7 +182,7 @@ class MODEL_EXPORT CResourceMonitor {
     //! Returns the sum of used memory plus any extra memory
     std::size_t totalMemory() const;
 
-    //! Returns the current physical memory of the process as reported by the system
+    //! Returns the current physical memory (rss) of the process as reported by the system
     std::size_t systemMemory() const;
 
     //! Returns the maximum physical memory of the processs as reported by the system

diff --git a/include/model/ModelTypes.h b/include/model/ModelTypes.h
@@ -719,9 +719,7 @@ enum EAssignmentMemoryBasis {
     E_AssignmentBasisUnknown = 0,           //!< Decision made in Java code
     E_AssignmentBasisModelMemoryLimit = 1,  //!< Use model memory limit
     E_AssignmentBasisCurrentModelBytes = 2, //!< Use current actual model size
-    E_AssignmentBasisPeakModelBytes = 3, //!< Use highest ever actual model size
-    E_AssignmentBasisSystemMemoryBytes = 4, //!< Use the current system memory size
-    E_AssignmentBasisMaxSystemMemoryBytes = 5 //!< Use the highest ever system memory size
+    E_AssignmentBasisPeakModelBytes = 3
 };
 
 //! Get a string description of \p assignmentMemoryBasis.

diff --git a/lib/api/unittest/CJsonOutputWriterTest.cc b/lib/api/unittest/CJsonOutputWriterTest.cc
@@ -1717,7 +1717,7 @@ BOOST_AUTO_TEST_CASE(testPersistNormalizer) {
     BOOST_TEST_REQUIRE(quantileState.contains("timestamp"));
 }
 
-BOOST_AUTO_TEST_CASE(testReportMemoryUsage) {
+BOOST_AUTO_TEST_CASE(testReportMemoryUsage)  {
     std::ostringstream sstream;
     {
         ml::core::CJsonOutputStreamWrapper outputStream(sstream);
@@ -1735,7 +1735,7 @@ BOOST_AUTO_TEST_CASE(testReportMemoryUsage) {
         resourceUsage.s_OverFields = 9;
         resourceUsage.s_AllocationFailures = 10;
         resourceUsage.s_MemoryStatus = ml::model_t::E_MemoryStatusHardLimit;
-        resourceUsage.s_AssignmentMemoryBasis = ml::model_t::E_AssignmentBasisSystemMemoryBytes;
+        resourceUsage.s_AssignmentMemoryBasis = ml::model_t::E_AssignmentBasisPeakModelBytes;
         resourceUsage.s_BucketStartTime = 11;
         resourceUsage.s_BytesExceeded = 12;
         resourceUsage.s_BytesMemoryLimit = 13;
@@ -1793,7 +1793,7 @@ BOOST_AUTO_TEST_CASE(testReportMemoryUsage) {
     BOOST_TEST_REQUIRE(sizeStats.contains("memory_status"));
     BOOST_REQUIRE_EQUAL("hard_limit", sizeStats.at("memory_status").as_string());
     BOOST_TEST_REQUIRE(sizeStats.contains("assignment_memory_basis"));
-    BOOST_REQUIRE_EQUAL("system_memory_bytes",
+    BOOST_REQUIRE_EQUAL("peak_model_bytes",
                         sizeStats.at("assignment_memory_basis").as_string());
     BOOST_TEST_REQUIRE(sizeStats.contains("log_time"));
     std::int64_t nowMs{ml::core::CTimeUtils::nowMs()};

diff --git a/lib/core/CProcessStats_Linux.cc b/lib/core/CProcessStats_Linux.cc
@@ -76,7 +76,7 @@ std::size_t CProcessStats::residentSetSize() {
         }
     }
 
-    CProgramCounters::counter(counter_t::E_TSADResidentSetSize) = rss;
+    CProgramCounters::counter(counter_t::E_TSADSystemMemoryUsage) = rss;
 
     return rss;
 }
@@ -92,7 +92,7 @@ std::size_t CProcessStats::maxResidentSetSize() {
     // ru_maxrss is in kilobytes
     auto maxRSS = static_cast<std::size_t>(rusage.ru_maxrss * 1024L);
 
-    CProgramCounters::counter(counter_t::E_TSADMaxResidentSetSize) = maxRSS;
+    CProgramCounters::counter(counter_t::E_TSADMaxSystemMemoryUsage) = maxRSS;
 
     return maxRSS;
 }

diff --git a/lib/core/CProcessStats_MacOSX.cc b/lib/core/CProcessStats_MacOSX.cc
@@ -23,7 +23,7 @@ namespace core {
 
 std::size_t CProcessStats::residentSetSize() {
     // not supported on osx
-    CProgramCounters::counter(counter_t::E_TSADResidentSetSize) = 0;
+    CProgramCounters::counter(counter_t::E_TSADSystemMemoryUsage) = 0;
     return 0;
 }
 
@@ -35,7 +35,7 @@ std::size_t CProcessStats::maxResidentSetSize() {
         return 0;
     }
     auto maxRSS = static_cast<std::size_t>(rusage.ru_maxrss);
-    CProgramCounters::counter(counter_t::E_TSADMaxResidentSetSize) = maxRSS;
+    CProgramCounters::counter(counter_t::E_TSADMaxSystemMemoryUsage) = maxRSS;
     // ru_maxrss is in bytes
     return maxRSS;
 }

diff --git a/lib/core/CProcessStats_Windows.cc b/lib/core/CProcessStats_Windows.cc
@@ -31,7 +31,7 @@ std::size_t CProcessStats::residentSetSize() {
 
     auto workingSetSize = static_cast<std::size_t>(stats.WorkingSetSize);
 
-    CProgramCounters::counter(counter_t::E_TSADResidentSetSize) = workingSetSize;
+    CProgramCounters::counter(counter_t::E_TSADSystemMemoryUsage) = workingSetSize;
 
     return workingSetSize;
 }
@@ -45,7 +45,7 @@ std::size_t CProcessStats::maxResidentSetSize() {
 
     auto peakWorkingSetSize = static_cast<std::size_t>(stats.PeakWorkingSetSize);
 
-    CProgramCounters::counter(counter_t::E_TSADMaxResidentSetSize) = peakWorkingSetSize;
+    CProgramCounters::counter(counter_t::E_TSADMaxSystemMemoryUsage) = peakWorkingSetSize;
 
     return peakWorkingSetSize;
 }

diff --git a/lib/model/ModelTypes.cc b/lib/model/ModelTypes.cc
@@ -1733,10 +1733,6 @@ std::string print(EAssignmentMemoryBasis assignmentMemoryBasis) {
         return "current_model_bytes";
     case E_AssignmentBasisPeakModelBytes:
         return "peak_model_bytes";
-    case E_AssignmentBasisSystemMemoryBytes:
-        return "system_memory_bytes";
-    case E_AssignmentBasisMaxSystemMemoryBytes:
-        return "max_system_memory_bytes";
     }
     return "-";
 }