Adjust NativeAllocationRegistry for new accounting

Provide a new NativeAllocationRegistry constructor that does not
require a size, assuming instead that all native allocation goes
through malloc() and the GC tracks native allocation anyway.

Use it in BigInt, Pattern, and Matcher, the traditional problem cases
for proper size tracking. We should use it in many other cases as well.
The fact that we're not yet doing so has the effect that we're currently
double-counting some allocations. However a mere factor of 2 error is
likely to be a huge improvement already over prior accounting.

Force the argument to setTargetHeapUtilization to be at least 0.1.
Very small numbers are not reasonable, and could cause overflows in ART.
(Needed by accounting cleanups in the companion ART CL.)

Bug: 111447610

Test: atest NativeAllocationRegistryTest (option 2)
Test: Built and booted AOSP. Ran 175-alloc-big-bignums.

Change-Id: Iad76e1efe0849ddf03834571f71a00e684e82858

8 files changed, 92 insertions, 40 deletions

diff --git a/libart/src/main/java/dalvik/system/VMRuntime.java b/libart/src/main/java/dalvik/system/VMRuntime.java
index 4d6b8c7e31..d2f0317b18 100644
--- a/libart/src/main/java/dalvik/system/VMRuntime.java
+++ b/libart/src/main/java/dalvik/system/VMRuntime.java
@@ -19,6 +19,7 @@ package dalvik.system;
 import dalvik.annotation.compat.UnsupportedAppUsage;
 import dalvik.annotation.optimization.FastNative;
 import java.lang.ref.FinalizerReference;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Consumer;
@@ -67,6 +68,13 @@ public final class VMRuntime {
 
     private int targetSdkVersion = SDK_VERSION_CUR_DEVELOPMENT;
 
+    // notifyNativeAllocationsInternal (below) should be called every notifyNativeInterval
+    // allocations. Initialized on demand to allow completely static class initialization.
+    private int notifyNativeInterval;
+
+    // Allocations since last call to native layer. See notifyNativeAllocation().
+    private final AtomicInteger allocationCount = new AtomicInteger(0);
+
     /**
      * Prevents this class from being instantiated.
      */
@@ -163,12 +171,14 @@ public final class VMRuntime {
     @libcore.api.CorePlatformApi
     public float setTargetHeapUtilization(float newTarget) {
         if (newTarget <= 0.0f || newTarget >= 1.0f) {
-            throw new IllegalArgumentException(newTarget +
-                    " out of range (0,1)");
+            throw new IllegalArgumentException(newTarget + " out of range (0,1)");
+        }
+        /* The native code assumes a value >= 0.1. Clamp it to that. */
+        if (newTarget < 0.1f) {
+            newTarget = 0.1f;
         }
-        /* Synchronize to make sure that only one thread gets
-         * a given "old" value if both update at the same time.
-         * Allows for reliable save-and-restore semantics.
+        /* Synchronize to make sure that only one thread gets a given "old" value if both
+         * update at the same time.  Allows for reliable save-and-restore semantics.
          */
         synchronized (this) {
             float oldTarget = getTargetHeapUtilization();
@@ -375,18 +385,69 @@ public final class VMRuntime {
      * function requests a concurrent GC. If the native bytes allocated exceeds a second higher
      * watermark, it is determined that the application is registering native allocations at an
      * unusually high rate and a GC is performed inside of the function to prevent memory usage
-     * from excessively increasing.
+     * from excessively increasing. Memory allocated via system malloc() should not be included
+     * in this count. If only malloced() memory is allocated, bytes should be zero.
+     * The argument must be the same as that later passed to registerNativeFree(), but may
+     * otherwise be approximate.
      */
     @UnsupportedAppUsage
     @libcore.api.CorePlatformApi
-    public native void registerNativeAllocation(int bytes);
+    public void registerNativeAllocation(int bytes) {
+        if (bytes == 0) {
+            notifyNativeAllocation();
+        } else {
+            registerNativeAllocationInternal(bytes);
+        }
+    }
+
+    private native void registerNativeAllocationInternal(int bytes);
 
     /**
      * Registers a native free by reducing the number of native bytes accounted for.
      */
     @UnsupportedAppUsage
     @libcore.api.CorePlatformApi
-    public native void registerNativeFree(int bytes);
+    public void registerNativeFree(int bytes) {
+        if (bytes != 0) {
+            registerNativeFreeInternal(bytes);
+        }
+    }
+
+    private native void registerNativeFreeInternal(int bytes);
+
+    /**
+     * Return the number of native objects that are reported by a single call to
+     * notifyNativeAllocation().
+     */
+    private static native int getNotifyNativeInterval();
+
+    /**
+     * Report a native malloc()-only allocation to the GC.
+     */
+    private void notifyNativeAllocation() {
+        // Minimize JNI calls by notifying once every notifyNativeInterval allocations.
+        // The native code cannot do anything without calling mallinfo(), which is too
+        // expensive to perform on every allocation. To avoid the JNI overhead on every
+        // allocation, we do the sampling here, rather than in native code.
+        // Initialize notifyNativeInterval carefully. Multiple initializations may race.
+        int myNotifyNativeInterval = notifyNativeInterval;
+        if (myNotifyNativeInterval == 0) {
+            // This can race. By Java rules, that's OK.
+            myNotifyNativeInterval = notifyNativeInterval = getNotifyNativeInterval();
+        }
+        // myNotifyNativeInterval is correct here. If another thread won the initial race,
+        // notifyNativeInterval may not be.
+        if (allocationCount.addAndGet(1) % myNotifyNativeInterval == 0) {
+            notifyNativeAllocationsInternal();
+        }
+    }
+
+    /**
+     * Report to the GC that roughly notifyNativeInterval native malloc()-based
+     * allocations have occurred since the last call to notifyNativeAllocationsInternal().
+     * Hints that we should check whether a GC is required.
+     */
+    private native void notifyNativeAllocationsInternal();
 
     /**
      * Wait for objects to be finalized.
diff --git a/luni/src/main/java/java/math/BigInt.java b/luni/src/main/java/java/math/BigInt.java
index 9faac8148b..de4cdd8e02 100644
--- a/luni/src/main/java/java/math/BigInt.java
+++ b/luni/src/main/java/java/math/BigInt.java
@@ -27,7 +27,7 @@ import libcore.util.NativeAllocationRegistry;
 final class BigInt {
 
     private static NativeAllocationRegistry registry = new NativeAllocationRegistry(
-            BigInt.class.getClassLoader(), NativeBN.getNativeFinalizer(), NativeBN.size());
+            BigInt.class.getClassLoader(), NativeBN.getNativeFinalizer());
 
     /* Fields used for the internal representation. */
     @ReachabilitySensitive
diff --git a/luni/src/main/java/java/math/NativeBN.java b/luni/src/main/java/java/math/NativeBN.java
index d269f2e27e..ab9b2e029f 100644
--- a/luni/src/main/java/java/math/NativeBN.java
+++ b/luni/src/main/java/java/math/NativeBN.java
@@ -133,10 +133,4 @@ final class NativeBN {
     public static native long getNativeFinalizer();
     // &BN_free
 
-    /** Returns the expected size of the native allocation for a BIGNUM.
-     */
-    public static long size() {
-        // 36 bytes is an empirically determined approximation.
-        return 36;
-    }
 }
diff --git a/luni/src/main/java/libcore/util/NativeAllocationRegistry.java b/luni/src/main/java/libcore/util/NativeAllocationRegistry.java
index c1b4fe0256..5c8e8313a7 100644
--- a/luni/src/main/java/libcore/util/NativeAllocationRegistry.java
+++ b/luni/src/main/java/libcore/util/NativeAllocationRegistry.java
@@ -65,13 +65,21 @@ public class NativeAllocationRegistry {
      * native bytes this kind of native allocation takes up. Different
      * NativeAllocationRegistrys must be used to register native allocations
      * with different estimated sizes, even if they use the same
-     * <code>freeFunction</code>.
+     * <code>freeFunction</code>. This is used to help inform the garbage
+     * collector about the possible need for collection. Memory allocated with
+     * native malloc is implicitly included, and ideally should not be included in this
+     * argument. For malloc-based native allocations that are expected to be under
+     * 100Kbytes or so, the NativeAllocationRegistry(ClassLoader,long) constructor is
+     * preferred.
+     * <p>
      * @param classLoader  ClassLoader that was used to load the native
      *                     library freeFunction belongs to.
      * @param freeFunction address of a native function used to free this
      *                     kind of native allocation
      * @param size         estimated size in bytes of this kind of native
-     *                     allocation
+     *                     allocation. Should ideally exclude memory allocated by system
+     *                     malloc. However including it will simply double-count it,
+     *                     typically resulting in slightly increased GC frequency.
      * @throws IllegalArgumentException If <code>size</code> is negative
      */
     @libcore.api.CorePlatformApi
@@ -79,13 +87,20 @@ public class NativeAllocationRegistry {
         if (size < 0) {
             throw new IllegalArgumentException("Invalid native allocation size: " + size);
         }
-
         this.classLoader = classLoader;
         this.freeFunction = freeFunction;
         this.size = size;
     }
 
     /**
+     * Equivalent to NativeAllocationRegistry(classLoader, freeFunction, 0).
+     * Should be used when all native memory is allocated via system malloc.
+     */
+    public NativeAllocationRegistry(ClassLoader classLoader, long freeFunction) {
+        this(classLoader, freeFunction, 0);
+    }
+
+    /**
      * Registers a new native allocation and associated Java object with the
      * runtime.
      * This NativeAllocationRegistry's <code>freeFunction</code> will
@@ -223,6 +238,8 @@ public class NativeAllocationRegistry {
         }
     }
 
+    // Inform the garbage collector of the allocation. The size = 0 case is optimized in
+    // VMRuntime.
     // TODO: Change the runtime to support passing the size as a long instead
     // of an int. For now, we clamp the size to fit.
     private static void registerNativeAllocation(long size) {
diff --git a/luni/src/main/native/java_util_regex_Matcher.cpp b/luni/src/main/native/java_util_regex_Matcher.cpp
index 47d8b80d64..048172234d 100644
--- a/luni/src/main/native/java_util_regex_Matcher.cpp
+++ b/luni/src/main/native/java_util_regex_Matcher.cpp
@@ -158,16 +158,6 @@ static jlong Matcher_getNativeFinalizer(JNIEnv*, jclass) {
     return reinterpret_cast<jlong>(&Matcher_free);
 }
 
-// Return a guess of the amount of native memory to be deallocated by a typical call to
-// Matcher_free().
-static jint Matcher_nativeSize(JNIEnv*, jclass) {
-    // This value can be tuned. 200 was found to cause performance issues (b/111141123) so 10000
-    // is being used instead. If the value is too small, there's no pressure to collect matchers,
-    // too large and we'll run GC too often.
-    // Also see b/111141123.
-    return 10000;
-}
-
 static jint Matcher_findImpl(JNIEnv* env, jclass, jlong addr, jint startIndex, jintArray offsets) {
     MatcherState* state = toMatcherState(addr);
     UBool result = state->matcher()->find(startIndex, state->status());
@@ -273,7 +263,6 @@ static JNINativeMethod gMethods[] = {
     NATIVE_METHOD(Matcher, hitEndImpl, "(J)Z"),
     NATIVE_METHOD(Matcher, lookingAtImpl, "(J[I)Z"),
     NATIVE_METHOD(Matcher, matchesImpl, "(J[I)Z"),
-    NATIVE_METHOD(Matcher, nativeSize, "()I"),
     NATIVE_METHOD(Matcher, openImpl, "(J)J"),
     NATIVE_METHOD(Matcher, requireEndImpl, "(J)Z"),
     NATIVE_METHOD(Matcher, setInputImpl, "(JLjava/lang/String;II)V"),
diff --git a/luni/src/main/native/java_util_regex_Pattern.cpp b/luni/src/main/native/java_util_regex_Pattern.cpp
index f4d23cb77d..ac13346c2e 100644
--- a/luni/src/main/native/java_util_regex_Pattern.cpp
+++ b/luni/src/main/native/java_util_regex_Pattern.cpp
@@ -75,12 +75,6 @@ static jlong Pattern_getNativeFinalizer(JNIEnv*, jclass) {
     return reinterpret_cast<jlong>(&Pattern_free);
 }
 
-// Return a guess of the amount of native memory to be deallocated by a typical call to
-// Pattern_free().
-static jint Pattern_nativeSize(JNIEnv*, jclass) {
-    return 500;  // Very rough guess based on a quick look at the implementation.
-}
-
 static jlong Pattern_compileImpl(JNIEnv* env, jclass, jstring javaRegex, jint flags) {
     flags |= UREGEX_ERROR_ON_UNKNOWN_ESCAPES;
 
@@ -103,7 +97,6 @@ static jlong Pattern_compileImpl(JNIEnv* env, jclass, jstring javaRegex, jint fl
 static JNINativeMethod gMethods[] = {
     NATIVE_METHOD(Pattern, compileImpl, "(Ljava/lang/String;I)J"),
     NATIVE_METHOD(Pattern, getNativeFinalizer, "()J"),
-    NATIVE_METHOD(Pattern, nativeSize, "()I"),
 };
 
 void register_java_util_regex_Pattern(JNIEnv* env) {
diff --git a/ojluni/src/main/java/java/util/regex/Matcher.java b/ojluni/src/main/java/java/util/regex/Matcher.java
index e92cb9dba0..edeecf32a9 100644
--- a/ojluni/src/main/java/java/util/regex/Matcher.java
+++ b/ojluni/src/main/java/java/util/regex/Matcher.java
@@ -150,7 +150,7 @@ public final class Matcher implements MatchResult {
     private Runnable nativeFinalizer;
 
     private static final NativeAllocationRegistry registry = new NativeAllocationRegistry(
-            Matcher.class.getClassLoader(), getNativeFinalizer(), nativeSize());
+            Matcher.class.getClassLoader(), getNativeFinalizer());
 
     /**
      * The index of the last position appended in a substitution.
@@ -1227,7 +1227,6 @@ public final class Matcher implements MatchResult {
     private static native boolean hitEndImpl(long addr);
     private static native boolean lookingAtImpl(long addr, int[] offsets);
     private static native boolean matchesImpl(long addr, int[] offsets);
-    private static native int nativeSize();
     private static native long openImpl(long patternAddr);
     private static native boolean requireEndImpl(long addr);
     private static native void setInputImpl(long addr, String s, int start, int end);
diff --git a/ojluni/src/main/java/java/util/regex/Pattern.java b/ojluni/src/main/java/java/util/regex/Pattern.java
index 04f4f80b20..437382baf8 100644
--- a/ojluni/src/main/java/java/util/regex/Pattern.java
+++ b/ojluni/src/main/java/java/util/regex/Pattern.java
@@ -949,7 +949,7 @@ public final class Pattern
     transient long address;
 
     private static final NativeAllocationRegistry registry = new NativeAllocationRegistry(
-            Pattern.class.getClassLoader(), getNativeFinalizer(), nativeSize());
+            Pattern.class.getClassLoader(), getNativeFinalizer());
     // END Android-changed: reimplement matching logic natively via ICU.
 
     /**
@@ -1435,7 +1435,6 @@ public final class Pattern
 
     private static native long compileImpl(String regex, int flags);
     private static native long getNativeFinalizer();
-    private static native int nativeSize();
     // END Android-changed: reimplement matching logic natively via ICU.
 
     /**