OSDN Git Service

cleanup redundant interfaces from iftable to avoid excessive LinearAlloc use
authorJohannes Rudolph <johannes.rudolph@gmail.com>
Fri, 13 Jan 2012 07:18:17 +0000 (08:18 +0100)
committerJohannes Rudolph <johannes.rudolph@gmail.com>
Fri, 13 Jan 2012 07:18:17 +0000 (08:18 +0100)
In deep interface hierarchies super-interfaces are recursively concatenated
to create the iftable. There was no checking for duplicated entries so that the
iftable could get pretty large with just a few layers of interfaces up to the
point where the LinearAlloc was exceeded completely.

This change scans the iftable linearly for existing entries before it adds another
one.

Issue: http://code.google.com/p/android/issues/detail?id=22586
Change-Id: Idb4a13ca7a52f390661629cf2539930242526876
Signed-off-by: Johannes Rudolph <johannes.rudolph@gmail.com>
tests/091-deep-interface-hierarchy/expected.txt [new file with mode: 0644]
tests/091-deep-interface-hierarchy/info.txt [new file with mode: 0644]
tests/091-deep-interface-hierarchy/src/Main.java [new file with mode: 0644]
vm/oo/Class.cpp

diff --git a/tests/091-deep-interface-hierarchy/expected.txt b/tests/091-deep-interface-hierarchy/expected.txt
new file mode 100644 (file)
index 0000000..33bcb02
--- /dev/null
@@ -0,0 +1 @@
+A new instance of Z was created successfully
diff --git a/tests/091-deep-interface-hierarchy/info.txt b/tests/091-deep-interface-hierarchy/info.txt
new file mode 100644 (file)
index 0000000..b62cec6
--- /dev/null
@@ -0,0 +1,4 @@
+Test class loading for deep interface hierarchies. The problem was that in a deep interface
+hierarchy super-interfaces were recursively concatenated without looking for duplicates.
+In cases like this can quickly lead to excessive LinearAlloc consumption due to more than linear
+duplication of iftables.
diff --git a/tests/091-deep-interface-hierarchy/src/Main.java b/tests/091-deep-interface-hierarchy/src/Main.java
new file mode 100644 (file)
index 0000000..8ab47f3
--- /dev/null
@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Create a hierarchy of interfaces to check if that overflows the LinearAlloc
+ * with iftable entries.
+ */
+public class Main {
+    interface A1 {}
+    interface A2 {}
+    interface A3 {}
+    interface A4 {}
+    interface A5 {}
+
+    interface B1 extends A1, A2, A3, A4, A5 {}
+    interface B2 extends A1, A2, A3, A4, A5 {}
+    interface B3 extends A1, A2, A3, A4, A5 {}
+    interface B4 extends A1, A2, A3, A4, A5 {}
+    interface B5 extends A1, A2, A3, A4, A5 {}
+
+    interface C1 extends B1, B2, B3, B4, B5 {}
+    interface C2 extends B1, B2, B3, B4, B5 {}
+    interface C3 extends B1, B2, B3, B4, B5 {}
+    interface C4 extends B1, B2, B3, B4, B5 {}
+    interface C5 extends B1, B2, B3, B4, B5 {}
+
+    interface D1 extends C1, C2, C3, C4, C5 {}
+    interface D2 extends C1, C2, C3, C4, C5 {}
+    interface D3 extends C1, C2, C3, C4, C5 {}
+    interface D4 extends C1, C2, C3, C4, C5 {}
+    interface D5 extends C1, C2, C3, C4, C5 {}
+
+    interface E1 extends D1, D2, D3, D4, D5 {}
+    interface E2 extends D1, D2, D3, D4, D5 {}
+    interface E3 extends D1, D2, D3, D4, D5 {}
+    interface E4 extends D1, D2, D3, D4, D5 {}
+    interface E5 extends D1, D2, D3, D4, D5 {}
+
+    interface F1 extends E1, E2, E3, E4, E5 {}
+    interface F2 extends E1, E2, E3, E4, E5 {}
+    interface F3 extends E1, E2, E3, E4, E5 {}
+    interface F4 extends E1, E2, E3, E4, E5 {}
+    interface F5 extends E1, E2, E3, E4, E5 {}
+
+    interface G1 extends F1, F2, F3, F4, F5 {}
+    interface G2 extends F1, F2, F3, F4, F5 {}
+    interface G3 extends F1, F2, F3, F4, F5 {}
+    interface G4 extends F1, F2, F3, F4, F5 {}
+    interface G5 extends F1, F2, F3, F4, F5 {}
+
+    interface H1 extends G1, G2, G3, G4, G5 {}
+    interface H2 extends G1, G2, G3, G4, G5 {}
+    interface H3 extends G1, G2, G3, G4, G5 {}
+    interface H4 extends G1, G2, G3, G4, G5 {}
+    interface H5 extends G1, G2, G3, G4, G5 {}
+
+    interface Z extends H1, H2, H3, H4, H5 {}
+
+    public static void main(String[] args) {
+        Z instance = new Z() {};
+        System.out.println("A new instance of Z was created successfully");
+    }
+}
index 85a3bbd..51600c9 100644 (file)
@@ -3071,53 +3071,48 @@ static bool createIftable(ClassObject* clazz)
 
         /* add entries for the interface's superinterfaces */
         for (int j = 0; j < interf->iftableCount; j++) {
-            clazz->iftable[idx++].clazz = interf->iftable[j].clazz;
+            int k;
+            ClassObject *cand;
+
+            cand = interf->iftable[j].clazz;
+
+            /*
+             * Check if this interface was already added and add only if new.
+             * This is to avoid a potential blowup in the number of
+             * interfaces for sufficiently complicated interface hierarchies.
+             * This has quadratic runtime in the number of interfaces.
+             * However, in common cases with little interface inheritance, this
+             * doesn't make much of a difference.
+             */
+            for (k = 0; k < idx; k++)
+                if (clazz->iftable[k].clazz == cand)
+                    break;
+
+            if (k == idx)
+                clazz->iftable[idx++].clazz = cand;
         }
     }
 
-    assert(idx == ifCount);
+    assert(idx <= ifCount);
 
+    /*
+     * Adjust the ifCount. We could reallocate the interface memory here,
+     * but it's probably not worth the effort, the important thing here
+     * is to avoid the interface blowup and keep the ifCount low.
+     */
     if (false) {
-        /*
-         * Remove anything redundant from our recent additions.  Note we have
-         * to traverse the recent adds when looking for duplicates, because
-         * it's possible the recent additions are self-redundant.  This
-         * reduces the memory footprint of classes with lots of inherited
-         * interfaces.
-         *
-         * (I don't know if this will cause problems later on when we're trying
-         * to find a static field.  It looks like the proper search order is
-         * (1) current class, (2) interfaces implemented by current class,
-         * (3) repeat with superclass.  A field implemented by an interface
-         * and by a superclass might come out wrong if the superclass also
-         * implements the interface.  The javac compiler will reject the
-         * situation as ambiguous, so the concern is somewhat artificial.)
-         *
-         * UPDATE: this makes ReferenceType.Interfaces difficult to implement,
-         * because it wants to return just the interfaces declared to be
-         * implemented directly by the class.  I'm excluding this code for now.
-         */
-        for (int i = superIfCount; i < ifCount; i++) {
-            for (int j = 0; j < ifCount; j++) {
-                if (i == j)
-                    continue;
-                if (clazz->iftable[i].clazz == clazz->iftable[j].clazz) {
-                    LOGVV("INTF: redundant interface %s in %s",
-                        clazz->iftable[i].clazz->descriptor,
-                        clazz->descriptor);
-
-                    if (i != ifCount-1)
-                        memmove(&clazz->iftable[i], &clazz->iftable[i+1],
-                            (ifCount - i -1) * sizeof(InterfaceEntry));
-                    ifCount--;
-                    i--;        // adjust for i++ above
-                    break;
-                }
-            }
+        if (idx != ifCount) {
+            int newIfCount = idx;
+            InterfaceEntry* oldmem = clazz->iftable;
+
+            clazz->iftable = (InterfaceEntry*) dvmLinearAlloc(clazz->classLoader,
+                            sizeof(InterfaceEntry) * newIfCount);
+            memcpy(clazz->iftable, oldmem, sizeof(InterfaceEntry) * newIfCount);
+            dvmLinearFree(clazz->classLoader, oldmem);
         }
-        LOGVV("INTF: class '%s' nodupes=%d", clazz->descriptor, ifCount);
-    } // if (false)
+    }
 
+    ifCount = idx;
     clazz->iftableCount = ifCount;
 
     /*