[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1456: MINIFICPP-1965 Add CMAKE flags to select malloc implementation

[GitBox <gi...@apache.org>]

fgerlits commented on code in PR #1456:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1456#discussion_r1054532014


##########
docker/test/integration/minifi/core/DockerTestCluster.py:
##########
@@ -318,3 +318,31 @@ def wait_for_metric_class_on_prometheus(self, metric_class, timeout_seconds):
 
     def wait_for_processor_metric_on_prometheus(self, metric_class, timeout_seconds, processor_name):
         return PrometheusChecker().wait_for_processor_metric_on_prometheus(metric_class, timeout_seconds, processor_name)
+
+    def wait_for_peak_memory_usage(self, minimum_peak_memory_usage: int, timeout_seconds: int) -> bool:
+        start_time = time.perf_counter()
+        current_peak_memory_usage = get_peak_memory_usage(get_minifi_pid())
+        if current_peak_memory_usage is None:
+            logging.warning("Failed to determine peak memory usage")
+            return False
+        while (time.perf_counter() - start_time) < timeout_seconds:
+            if current_peak_memory_usage > minimum_peak_memory_usage:
+                return True
+            current_peak_memory_usage = get_peak_memory_usage(get_minifi_pid())
+            time.sleep(1)

Review Comment:
   why do we check `current_peak_memory_usage` twice?  would this work?
   ```suggestion
           while (time.perf_counter() - start_time) < timeout_seconds:
               current_peak_memory_usage = get_peak_memory_usage(get_minifi_pid())
               if current_peak_memory_usage is None:
                   logging.warning("Failed to determine peak memory usage")
                   return False
               if current_peak_memory_usage > minimum_peak_memory_usage:
                   return True
               time.sleep(1)
   ```
   (similarly in the other function below)



##########
CMakeLists.txt:
##########
@@ -196,14 +196,26 @@ set(PASSTHROUGH_CMAKE_ARGS -DANDROID_ABI=${ANDROID_ABI}
     -G${CMAKE_GENERATOR}
     )
 
-# jemalloc
+if(CUSTOM_MALLOC)
+    if (CUSTOM_MALLOC STREQUAL jemalloc)
+        include(BundledJemalloc)
+        use_bundled_jemalloc(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR})
+        set(CUSTOM_MALLOC_LIB JeMalloc::JeMalloc)
+    elseif (CUSTOM_MALLOC STREQUAL mimalloc)
+        include(MiMalloc)
+        set(CUSTOM_MALLOC_LIB mimalloc)
+    elseif (CUSTOM_MALLOC STREQUAL rpmalloc)
+        include(RpMalloc)
+        set(CUSTOM_MALLOC_LIB rpmalloc)
+    else()
+        message(ERROR "Invalid CUSTOM_MALLOC")

Review Comment:
   The docs I have found have only `FATAL_ERROR` and `SEND_ERROR` but no plain `ERROR`: https://cmake.org/cmake/help/latest/command/message.html  Is `ERROR` an alias to `FATAL_ERROR`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org