Suggestions from Code Review

Author: Lillian Zhang

src/main/java/org/truffleruby/core/GCNodes.java

@@ -15,6 +15,8 @@
 import java.lang.management.MemoryUsage;
 import java.time.Duration;
 
+import com.oracle.truffle.api.dsl.Cached;
+import org.jcodings.specific.UTF8Encoding;
 import org.truffleruby.SuppressFBWarnings;
 import org.truffleruby.builtins.CoreMethod;
 import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
@@ -26,6 +28,8 @@
 import org.truffleruby.builtins.PrimitiveArrayArgumentsNode;
 import org.truffleruby.collections.WeakValueCache;
 import org.truffleruby.core.array.RubyArray;
+import org.truffleruby.core.rope.CodeRange;
+import org.truffleruby.core.string.StringNodes;
 import org.truffleruby.language.SafepointManager;
 import org.truffleruby.language.control.RaiseException;
 
@@ -135,7 +139,8 @@ public static abstract class GCStatPrimitiveNode extends PrimitiveArrayArguments
 
         @TruffleBoundary
         @Specialization
-        protected RubyArray stat() {
+        protected RubyArray stat(
+                @Cached StringNodes.MakeStringNode makeStringNode) {
             long time = 0;
             int count = 0;
             int minorCount = 0;
@@ -173,13 +178,12 @@ protected RubyArray stat() {
 
             // Get memory usage values from relevant memory pools (2-3 / ~8 are relevant)
             memoryPools = new Object[memoryPoolNames.length];
-            for (MemoryPoolMXBean bean : ManagementFactory.getMemoryPoolMXBeans()) {
-                if (bean.getName().equals(memoryPoolNames[0])) {
-                    memoryPools[0] = beanToArray(bean);
-                } else if (bean.getName().equals(memoryPoolNames[1])) {
-                    memoryPools[1] = beanToArray(bean);
-                } else if (memoryPoolNames.length == 3 && bean.getName().equals(memoryPoolNames[2])) {
-                    memoryPools[2] = beanToArray(bean);
+            for (int i = 0; i < memoryPoolNames.length; i++) {
+                String memoryPoolName = memoryPoolNames[i];
+                for (MemoryPoolMXBean bean : ManagementFactory.getMemoryPoolMXBeans()) {
+                    if (bean.getName().equals(memoryPoolName)) {
+                        memoryPools[i] = beanToArray(bean);
+                    }
                 }
             }
 
@@ -188,7 +192,8 @@ protected RubyArray stat() {
             // Use an object array instead because otherwise ArrayHelpers.java line 51 complains
             Object[] memoryPoolNamesCast = new Object[memoryPoolNames.length];
             for (int i = 0; i < memoryPoolNames.length; i++) {
-                memoryPoolNamesCast[i] = memoryPoolNames[i];
+                memoryPoolNamesCast[i] = makeStringNode
+                        .executeMake(memoryPoolNames[i], UTF8Encoding.INSTANCE, CodeRange.CR_UNKNOWN);
             }
 
 
@@ -205,16 +210,24 @@ protected RubyArray stat() {
                             createArray(memoryPools) });
         }
 
-        protected RubyArray usageToArray(MemoryUsage usage) {
-            return createArray(new Object[]{ usage.getCommitted(), usage.getInit(), usage.getMax(), usage.getUsed() });
-        }
-
         protected RubyArray beanToArray(MemoryPoolMXBean bean) {
+            MemoryUsage usage = bean.getUsage();
+            MemoryUsage peak = bean.getPeakUsage();
+            MemoryUsage last = bean.getCollectionUsage();
             return createArray(
-                    new Object[]{
-                            usageToArray(bean.getUsage()),
-                            usageToArray(bean.getPeakUsage()),
-                            usageToArray(bean.getCollectionUsage()) });
+                    new long[]{
+                            usage.getCommitted(),
+                            usage.getInit(),
+                            usage.getMax(),
+                            usage.getUsed(),
+                            peak.getCommitted(),
+                            peak.getInit(),
+                            peak.getMax(),
+                            peak.getUsed(),
+                            last.getCommitted(),
+                            last.getInit(),
+                            last.getMax(),
+                            last.getUsed() });
         }
 
     }

src/main/ruby/truffleruby/core/gc.rb

@@ -76,7 +76,7 @@ def garbage_collect
   end
 
   def self.stat(option = nil)
-    time, count, minor_count, major_count, unknown_count, commited, used, memory_pool_names, memory_pool_info = Primitive.gc_stat()
+    time, count, minor_count, major_count, unknown_count, committed, used, memory_pool_names, memory_pool_info = Primitive.gc_stat()
 
     # Initialize stat for statistics that come from memory pools, and populate it with some final stats
     stat = {
@@ -97,30 +97,31 @@ def self.stat(option = nil)
       :last_init => 0.0,
       :last_max => 0.0,
       :last_used => 0.0,
-      :heap_available_slots => commited, # should be the same as the calculated commited
+      :heap_available_slots => committed, # should be the same as the calculated committed
       :heap_live_slots => used, # should be the same as the calculated used
-      :heap_free_slots => commited - used,
+      :heap_free_slots => committed - used,
     }
 
     (0...memory_pool_names.length).each do |i|
       # Populate memory pool specific stats
-      stat[Truffle::Interop.to_string(memory_pool_names[i])] = {
-        :committed => memory_pool_info[i][0][0],
-        :init => memory_pool_info[i][0][1],
-        :max => memory_pool_info[i][0][2],
-        :used => memory_pool_info[i][0][3],
-        :peak_committed => memory_pool_info[i][1][0],
-        :peak_init => memory_pool_info[i][1][1],
-        :peak_max => memory_pool_info[i][1][2],
-        :peak_used => memory_pool_info[i][1][3],
-        :last_committed => memory_pool_info[i][2][0],
-        :last_init => memory_pool_info[i][2][1],
-        :last_max => memory_pool_info[i][2][2],
-        :last_used => memory_pool_info[i][2][3],
+      info = memory_pool_info[i]
+      stat[memory_pool_names[i]] = {
+        :committed => info[0],
+        :init => info[1],
+        :max => info[2],
+        :used => info[3],
+        :peak_committed => info[4],
+        :peak_init => info[5],
+        :peak_max => info[6],
+        :peak_used => info[7],
+        :last_committed => info[8],
+        :last_init => info[9],
+        :last_max => info[10],
+        :last_used => info[11],
       }
 
       # Calculate stats across memory pools
-      stat[Truffle::Interop.to_string(memory_pool_names[i])].each_pair do |key, value|
+      stat[memory_pool_names[i]].each_pair do |key, value|
         stat[key] += value
       end
     end