Patch: Add methods to GCInfo to allow GC_free_space_divisor to be manipulated.

Patch: Add methods to GCInfo to allow GC_free_space_divisor to be manipulated.

[David Daney]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

Ever since the GC's internal symbols quit being exported by libgcj.so, 
it has been impossible to change the GC_free_space_divisor.  We need to 
be able to tune this value in restricted memory environments to prevent 
OutOfMemoryErrors.

This patch adds a couple of methods to the new gnu.gcj.util.GCInfo class 
that allow the GC_free_space_divisor parameter to be tuned.

Tested on x86_64-pc-linux-gnu.

OK to commit?

2007-10-18  David Daney  <ddaney@avtrex.com>

    * gnu/gcj/util/GCInfo.java (checkPermission): Renamed to ...
    (checkDumpPermission): ... this and updated references throughout.
    (checkParametersPermission, getGCFreeSpaceDivisor,
    setGCFreeSpaceDivisor): New methods.
    (getGCFreeSpaceDivisor0, setGCFreeSpaceDivisor0): Declare.
    * gnu/gcj/util/natGCInfo.cc: Move #includes outside of ifdef block.
    (getGCFreeSpaceDivisor0, setGCFreeSpaceDivisor0): New methods.
    * gnu/gcj/util/GCInfo.h: Regenerate.
    * classpath/lib/gnu/gcj/util/GCInfo.class: Ditto.


[-- Attachment #2: gcinfo.patch --]
[-- Type: text/x-patch, Size: 3912 bytes --]

Index: gnu/gcj/util/GCInfo.java
===================================================================
--- gnu/gcj/util/GCInfo.java	(revision 129428)
+++ gnu/gcj/util/GCInfo.java	(working copy)
@@ -19,13 +19,60 @@ public class GCInfo
    * @throws SecurityException if there is a SecurityManager installed
    * and UtilPermission("dumpHeap") is not granted.
    */
-  private static void checkPermission()
+  private static void checkDumpPermission()
   {
     SecurityManager sm = System.getSecurityManager();
     if (sm != null)
       sm.checkPermission(new UtilPermission("dumpHeap"));
   }
-  
+
+  /**
+   * @throws SecurityException if there is a SecurityManager installed
+   * and UtilPermission("dumpHeap") is not granted.
+   */
+  private static void checkParametersPermission()
+  {
+    SecurityManager sm = System.getSecurityManager();
+    if (sm != null)
+      sm.checkPermission(new UtilPermission("changeGCParameters"));
+  }
+
+  /**
+   * Get the garbage collector's GC_free_space_divisor parameter.
+   *
+   * @return the divisor.
+   *
+   * @throws SecurityException if there is a SecurityManager installed
+   * and UtilPermission("changeGCParameters") is not granted.
+   */
+  public static int getGCFreeSpaceDivisor()
+  {
+    checkParametersPermission();
+    return getGCFreeSpaceDivisor0();
+  }
+
+  private static native int getGCFreeSpaceDivisor0();
+
+  /**
+   * Set the garbage collector's GC_free_space_divisor parameter.  The
+   * divisor may be set to a value greater than zero.
+   *
+   * @param divisor The new divisor value.
+   *
+   * @throws SecurityException if there is a SecurityManager installed
+   * and UtilPermission("changeGCParameters") is not granted.
+   *
+   * @throws IllegalArgumentException if divisor is not greater than zero.
+   */
+  public static void setGCFreeSpaceDivisor(int divisor)
+  {
+    checkParametersPermission();
+    if (divisor < 1)
+      throw new IllegalArgumentException();
+    setGCFreeSpaceDivisor0(divisor);
+  }
+
+  private static native void setGCFreeSpaceDivisor0(int divisor);
 
   /**
    * Dump a description of the heap state.
@@ -37,12 +84,11 @@ public class GCInfo
    */
   public static synchronized void dump(String name)
   {
-    checkPermission();
+    checkDumpPermission();
     dump0(name);
   }
-  
-  private static native void dump0(String name);
 
+  private static native void dump0(String name);
 
   /**
    * Create a heap dump.
@@ -54,10 +100,10 @@ public class GCInfo
    */
   public static synchronized void enumerate(String namePrefix)
   {
-    checkPermission();
+    checkDumpPermission();
     enumerate0(namePrefix);
   }
-  
+
   private static native void enumerate0(String namePrefix);
 
   /**
@@ -71,9 +117,9 @@ public class GCInfo
    */
   public static synchronized void setOOMDump(String namePrefix)
   {
-    checkPermission();
+    checkDumpPermission();
     setOOMDump0(namePrefix);
   }
-  
+
   private static native void setOOMDump0(String namePrefix);
 }

Index: gnu/gcj/util/natGCInfo.cc
===================================================================
--- gnu/gcj/util/natGCInfo.cc	(revision 129428)
+++ gnu/gcj/util/natGCInfo.cc	(working copy)
@@ -14,10 +14,6 @@
 
 #include <gnu/gcj/util/GCInfo.h>
 
-#ifdef HAVE_PROC_SELF_MAPS
-//
-// If /proc/self/maps does not exist we assume we are doomed and do nothing.
-//
 #include <stdio.h>
 #include <stdlib.h>
 #include <fcntl.h>
@@ -60,9 +56,10 @@ extern "C" {
   extern int GC_gcj_debug_kind;
 }
 
-#endif
-
 #ifdef HAVE_PROC_SELF_MAPS
+//
+// If /proc/self/maps does not exist we assume we are doomed and do nothing.
+//
 
 static int gc_ok = 1;
 
@@ -456,3 +453,14 @@ void
 
 #endif // HAVE_PROC_SELF_MAPS
 
+jint
+::gnu::gcj::util::GCInfo::getGCFreeSpaceDivisor0()
+{
+  return GC_free_space_divisor;
+}
+
+void
+::gnu::gcj::util::GCInfo::setGCFreeSpaceDivisor0(jint d)
+{
+  GC_free_space_divisor = d;
+}

Re: Patch: Add methods to GCInfo to allow GC_free_space_divisor to be manipulated.

[David Daney]

David Daney wrote:
> +  /**
> +   * @throws SecurityException if there is a SecurityManager installed
> +   * and UtilPermission("dumpHeap") is not granted.
> +   */
> +  private static void checkParametersPermission()
> +  {
> +    SecurityManager sm = System.getSecurityManager();
> +    if (sm != null)
> +      sm.checkPermission(new UtilPermission("changeGCParameters"));
> +  }

OK, that javadoc is incorrect.  I will fix it to read "changeGCParameters".

David Daney

Re: Patch: Add methods to GCInfo to allow GC_free_space_divisor to be manipulated.

[David Daney]

David Daney wrote:
> Ever since the GC's internal symbols quit being exported by libgcj.so, 
> it has been impossible to change the GC_free_space_divisor.  We need 
> to be able to tune this value in restricted memory environments to 
> prevent OutOfMemoryErrors.
>
> This patch adds a couple of methods to the new gnu.gcj.util.GCInfo 
> class that allow the GC_free_space_divisor parameter to be tuned.
>
> Tested on x86_64-pc-linux-gnu.
>
> OK to commit?
>
> 2007-10-18  David Daney  <ddaney@avtrex.com>
>
>    * gnu/gcj/util/GCInfo.java (checkPermission): Renamed to ...
>    (checkDumpPermission): ... this and updated references throughout.
>    (checkParametersPermission, getGCFreeSpaceDivisor,
>    setGCFreeSpaceDivisor): New methods.
>    (getGCFreeSpaceDivisor0, setGCFreeSpaceDivisor0): Declare.
>    * gnu/gcj/util/natGCInfo.cc: Move #includes outside of ifdef block.
>    (getGCFreeSpaceDivisor0, setGCFreeSpaceDivisor0): New methods.
>    * gnu/gcj/util/GCInfo.h: Regenerate.
>    * classpath/lib/gnu/gcj/util/GCInfo.class: Ditto.
>
After more thought, I think it is a better idea to add a _Jv_???? method
to set the free_space_divisor.  We really want to be able to set the
divisor before any memory is allocated by the GC.  If we set the divisor
via java code, there will already have been many allocations,
potentially using up too much memory.  I will work up a patch to do that
instead.

Consider this patch withdrawn.

David Daney