Improve zygote heap sharing.
Lots of ClassObject structures are unshared [and thus make whole pages
unshared]. A big contributor to this is the initializing class loader
logic, which has two writable words in a ClassObject and does indeed
seem to write them a lot.
Class which are created "early" have a low serial number and are
probably being created by the zygote, and would benefit from better
sharing. This change move the two words for classes with a low serial
number to a global, making the two words in those ClassObjects unused,
and thus more likely to be shared across apps.
Measured increase in sharing is c 100K per app, at a cost of c 32K
increase in unshared globals.
This might be better as a Set<Pair<Class,ClassLoader>> or
something. Many ClassObjects have zero loaders, and the typical count
seems to be very small. It should be possible to remove the two words
from the ClassObject and at the same time have a smaller global data
structure.
Respond to reviewer's comments.
Moved the "external" InitiatingLoaderList[] from gDvm to allocated storage.
Made the warning for the "const" go away.
Remove "vm/Init.c" from the commit
Revert Globals.h to not contain ZYGOTE_CLASS_CUTOFF
Use calloc rather than malloc/clear
diff --git a/vm/oo/Class.c b/vm/oo/Class.c
index c7ab763..47e2a87 100644
--- a/vm/oo/Class.c
+++ b/vm/oo/Class.c
@@ -151,6 +151,9 @@
may not be worth the performance hit.
*/
+#define INITIAL_CLASS_SERIAL_NUMBER 0x50000000
+#define ZYGOTE_CLASS_CUTOFF 2000
+
static ClassPathEntry* processClassPath(const char* pathStr, bool isBootstrap);
static void freeCpeArray(ClassPathEntry* cpe);
@@ -293,8 +296,15 @@
* Class serial number. We start with a high value to make it distinct
* in binary dumps (e.g. hprof).
*/
- gDvm.classSerialNumber = 0x50000000;
+ gDvm.classSerialNumber = INITIAL_CLASS_SERIAL_NUMBER;
+ /* Set up the table we'll use for tracking initiating loaders for
+ * early classes.
+ * If it's NULL, we just fall back to the InitiatingLoaderList in the
+ * ClassObject, so it's not fatal to fail this allocation.
+ */
+ gDvm.initiatingLoaderList =
+ calloc(ZYGOTE_CLASS_CUTOFF, sizeof(InitiatingLoaderList));
/* This placeholder class is used while a ClassObject is
* loading/linking so those not in the know can still say
@@ -346,6 +356,8 @@
gDvm.bootClassPath = NULL;
dvmLinearAllocDestroy(NULL);
+
+ free(gDvm.initiatingLoaderList);
}
@@ -797,6 +809,18 @@
#define kInitLoaderInc 4 /* must be power of 2 */
+static InitiatingLoaderList *dvmGetInitiatingLoaderList(ClassObject* clazz)
+{
+ assert(clazz->serialNumber > INITIAL_CLASS_SERIAL_NUMBER);
+ int classIndex = clazz->serialNumber-INITIAL_CLASS_SERIAL_NUMBER;
+ if (gDvm.initiatingLoaderList != NULL &&
+ classIndex < ZYGOTE_CLASS_CUTOFF) {
+ return &(gDvm.initiatingLoaderList[classIndex]);
+ } else {
+ return &(clazz->initiatingLoaderList);
+ }
+}
+
/*
* Determine if "loader" appears in clazz' initiating loader list.
*
@@ -820,9 +844,13 @@
/*
* Scan the list for a match. The list is expected to be short.
*/
+ /* Cast to remove the const from clazz, but use const loaderList */
+ ClassObject* nonConstClazz = (ClassObject*) clazz;
+ const InitiatingLoaderList *loaderList =
+ dvmGetInitiatingLoaderList(nonConstClazz);
int i;
- for (i = clazz->initiatingLoaderCount-1; i >= 0; --i) {
- if (clazz->initiatingLoaders[i] == loader) {
+ for (i = loaderList->initiatingLoaderCount-1; i >= 0; --i) {
+ if (loaderList->initiatingLoaders[i] == loader) {
//LOGI("+++ found initiating match %p in %s\n",
// loader, clazz->descriptor);
return true;
@@ -854,10 +882,10 @@
* pretty minor, and probably outweighs the O(n^2) hit for
* checking before every add, so we may not want to do this.
*/
- if (false && dvmLoaderInInitiatingList(clazz, loader)) {
- LOGW("WOW: simultaneous add of initiating class loader\n");
- goto bail_unlock;
- }
+ //if (dvmLoaderInInitiatingList(clazz, loader)) {
+ // LOGW("WOW: simultaneous add of initiating class loader\n");
+ // goto bail_unlock;
+ //}
/*
* The list never shrinks, so we just keep a count of the
@@ -867,25 +895,26 @@
* The pointer is initially NULL, so we *do* want to call realloc
* when count==0.
*/
- if ((clazz->initiatingLoaderCount & (kInitLoaderInc-1)) == 0) {
+ InitiatingLoaderList *loaderList = dvmGetInitiatingLoaderList(clazz);
+ if ((loaderList->initiatingLoaderCount & (kInitLoaderInc-1)) == 0) {
Object** newList;
- newList = (Object**) realloc(clazz->initiatingLoaders,
- (clazz->initiatingLoaderCount + kInitLoaderInc)
+ newList = (Object**) realloc(loaderList->initiatingLoaders,
+ (loaderList->initiatingLoaderCount + kInitLoaderInc)
* sizeof(Object*));
if (newList == NULL) {
/* this is mainly a cache, so it's not the EotW */
assert(false);
goto bail_unlock;
}
- clazz->initiatingLoaders = newList;
+ loaderList->initiatingLoaders = newList;
//LOGI("Expanded init list to %d (%s)\n",
- // clazz->initiatingLoaderCount+kInitLoaderInc,
+ // loaderList->initiatingLoaderCount+kInitLoaderInc,
// clazz->descriptor);
}
-
- clazz->initiatingLoaders[clazz->initiatingLoaderCount++] = loader;
+ loaderList->initiatingLoaders[loaderList->initiatingLoaderCount++] =
+ loader;
bail_unlock:
dvmHashTableUnlock(gDvm.loadedClasses);
@@ -1889,8 +1918,9 @@
dvmLinearFree(clazz->classLoader, virtualMethods);
}
- clazz->initiatingLoaderCount = -1;
- NULL_AND_FREE(clazz->initiatingLoaders);
+ InitiatingLoaderList *loaderList = dvmGetInitiatingLoaderList(clazz);
+ loaderList->initiatingLoaderCount = -1;
+ NULL_AND_FREE(loaderList->initiatingLoaders);
clazz->interfaceCount = -1;
NULL_AND_LINEAR_FREE(clazz->interfaces);