8144527: NewSizeThreadIncrease would make an overflow
authorsangheki
Tue, 05 Jan 2016 17:05:13 -0800
changeset 35458 71a08884be5f
parent 35457 e426a2aee43a
child 35459 e5554d76304a
8144527: NewSizeThreadIncrease would make an overflow Summary: Revert to previous value if NewSizeThreadIncrease related calculation overflows Reviewed-by: jwilhelm, mgerdin, mchernov
hotspot/src/share/vm/gc/serial/defNewGeneration.cpp
hotspot/src/share/vm/gc/serial/defNewGeneration.hpp
hotspot/test/gc/arguments/TestNewSizeThreadIncrease.java
--- a/hotspot/src/share/vm/gc/serial/defNewGeneration.cpp	Wed Jan 06 14:54:24 2016 +0000
+++ b/hotspot/src/share/vm/gc/serial/defNewGeneration.cpp	Tue Jan 05 17:05:13 2016 -0800
@@ -364,6 +364,36 @@
   return success;
 }
 
+size_t DefNewGeneration::adjust_for_thread_increase(size_t new_size_candidate,
+                                                    size_t new_size_before,
+                                                    size_t alignment) const {
+  size_t desired_new_size = new_size_before;
+
+  if (NewSizeThreadIncrease > 0) {
+    int threads_count;
+    size_t thread_increase_size = 0;
+
+    // 1. Check an overflow at 'threads_count * NewSizeThreadIncrease'.
+    threads_count = Threads::number_of_non_daemon_threads();
+    if (NewSizeThreadIncrease <= max_uintx / threads_count) {
+      thread_increase_size = threads_count * NewSizeThreadIncrease;
+
+      // 2. Check an overflow at 'new_size_candidate + thread_increase_size'.
+      if (new_size_candidate <= max_uintx - thread_increase_size) {
+        new_size_candidate += thread_increase_size;
+
+        // 3. Check an overflow at 'align_size_up'.
+        size_t aligned_max = ((max_uintx - alignment) & ~(alignment-1));
+        if (new_size_candidate <= aligned_max) {
+          desired_new_size = align_size_up(new_size_candidate, alignment);
+        }
+      }
+    }
+  }
+
+  return desired_new_size;
+}
+
 void DefNewGeneration::compute_new_size() {
   // This is called after a GC that includes the old generation, so from-space
   // will normally be empty.
@@ -385,12 +415,13 @@
   // All space sizes must be multiples of Generation::GenGrain.
   size_t alignment = Generation::GenGrain;
 
-  // Compute desired new generation size based on NewRatio and
-  // NewSizeThreadIncrease
-  size_t desired_new_size = old_size/NewRatio;
-  int threads_count = Threads::number_of_non_daemon_threads();
-  size_t thread_increase_size = threads_count * NewSizeThreadIncrease;
-  desired_new_size = align_size_up(desired_new_size + thread_increase_size, alignment);
+  int threads_count = 0;
+  size_t thread_increase_size = 0;
+
+  size_t new_size_candidate = old_size / NewRatio;
+  // Compute desired new generation size based on NewRatio and NewSizeThreadIncrease
+  // and reverts to previous value if any overflow happens
+  size_t desired_new_size = adjust_for_thread_increase(new_size_candidate, new_size_before, alignment);
 
   // Adjust new generation size
   desired_new_size = MAX2(MIN2(desired_new_size, max_new_size), min_new_size);
--- a/hotspot/src/share/vm/gc/serial/defNewGeneration.hpp	Wed Jan 06 14:54:24 2016 +0000
+++ b/hotspot/src/share/vm/gc/serial/defNewGeneration.hpp	Tue Jan 05 17:05:13 2016 -0800
@@ -354,6 +354,14 @@
   void compute_space_boundaries(uintx minimum_eden_size,
                                 bool clear_space,
                                 bool mangle_space);
+
+  // Return adjusted new size for NewSizeThreadIncrease.
+  // If any overflow happens, revert to previous new size.
+  size_t adjust_for_thread_increase(size_t new_size_candidate,
+                                    size_t new_size_before,
+                                    size_t alignment) const;
+
+
   // Scavenge support
   void swap_spaces();
 };
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/gc/arguments/TestNewSizeThreadIncrease.java	Tue Jan 05 17:05:13 2016 -0800
@@ -0,0 +1,123 @@
+/*
+* Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+*
+* This code is free software; you can redistribute it and/or modify it
+* under the terms of the GNU General Public License version 2 only, as
+* published by the Free Software Foundation.
+*
+* This code is distributed in the hope that it will be useful, but WITHOUT
+* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+* FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+* version 2 for more details (a copy is included in the LICENSE file that
+* accompanied this code).
+*
+* You should have received a copy of the GNU General Public License version
+* 2 along with this work; if not, write to the Free Software Foundation,
+* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+*
+* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+* or visit www.oracle.com if you need additional information or have any
+* questions.
+*/
+
+/*
+ * @test TestNewSizeThreadIncrease
+ * @key gc
+ * @bug 8144527
+ * @summary Tests argument processing for NewSizeThreadIncrease
+ * @library /testlibrary
+ * @requires vm.gc=="Serial" | vm.gc=="null"
+ * @modules java.base/sun.misc
+ *          java.management
+ */
+
+
+import jdk.test.lib.*;
+
+// Range of NewSizeThreadIncrease is 0 ~ max_uintx.
+// Total of 5 threads will be created (1 GCTest thread and 4 TestThread).
+public class TestNewSizeThreadIncrease {
+  static final String VALID_VALUE = "2097152"; // 2MB
+
+  // This value will make an overflow of 'thread count * NewSizeThreadIncrease' at DefNewGeneration::compute_new_size().
+  // = (max_uintx / 5) + 1, = (18446744073709551615 / 5) + 1
+  static String INVALID_VALUE_1 = "3689348814741910324";
+
+  // This string is contained when compute_new_size() expands or shrinks.
+  static final String LOG_NEWSIZE_CHANGED = "New generation size ";
+
+  public static void main(String[] args) throws Exception {
+    if (Platform.is32bit()) {
+      // (max_uintx / 5) + 1, 4294967295/5 + 1
+      INVALID_VALUE_1 = "858993460";
+    }
+
+    // New size will be applied as NewSizeThreadIncrease is small enough to expand.
+    runNewSizeThreadIncreaseTest(VALID_VALUE, true);
+
+    // New size will be ignored as 'thread count * NewSizeThreadIncrease' overflows.
+    runNewSizeThreadIncreaseTest(INVALID_VALUE_1, false);
+  }
+
+  static void runNewSizeThreadIncreaseTest(String expectedValue, boolean isNewsizeChanged) throws Exception {
+    ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-XX:+UseSerialGC",
+                                                              "-Xms96M",
+                                                              "-Xmx128M",
+                                                              "-XX:NewRatio=2",
+                                                              "-Xlog:gc+heap+ergo=debug",
+                                                              "-XX:NewSizeThreadIncrease="+expectedValue,
+                                                              GCTest.class.getName());
+    OutputAnalyzer output = new OutputAnalyzer(pb.start());
+
+    output.shouldHaveExitValue(0);
+
+    if (isNewsizeChanged) {
+      output.shouldContain(LOG_NEWSIZE_CHANGED);
+    } else {
+      output.shouldNotContain(LOG_NEWSIZE_CHANGED);
+    }
+  }
+
+  static class GCTest {
+
+    static final int MAX_THREADS_COUNT = 4;
+    static TestThread threads[] = new TestThread[MAX_THREADS_COUNT];
+
+    public static void main(String[] args) {
+
+      System.out.println("Creating garbage");
+
+      for (int i=0; i<MAX_THREADS_COUNT; i++) {
+        threads[i] = new TestThread();
+        threads[i].start();
+      }
+
+      System.gc();
+
+      for (int i=0; i<MAX_THREADS_COUNT; i++) {
+        threads[i].stopRunning();
+      }
+
+      System.out.println("Done");
+    }
+
+    private static class TestThread extends Thread {
+
+      volatile boolean isRunning = true;
+
+      public void run() {
+        while (isRunning == true) {
+          try {
+            Thread.sleep(10);
+          } catch (Throwable t) {
+          }
+        }
+      }
+
+      public void stopRunning() {
+        isRunning = false;
+      }
+    }
+  }
+}