1

I have a cache implemented in c++ using multimap and then I look up the cache and return the matching integer values as an array to the Java code.

The problem I am facing is, the Old generation progressively gets completely full and triggers continuous full GC from thereon. I found out, that, just returning a static array is causing the Old generation to fillup periodically and trigger a full GC. Eventually at a high load system will heap dump.

The code is like below Java

for(JavaObject o: javaOjctes){
     int[] values = cache.get(o.getkey1,o.getkey2,o.getkeyType[]);
      //further processing
   }

and on C++ side I have CacheImpl.cpp

JNIEXPORT jintArray JNICALL Java_com_test_cache_CacheService_get(JNIEnv *env, jobject thisObject, jstring key1,jstring key2,jobjectArray keyTypes){

const char *key1Value = (env)->GetStringUTFChars(key1,NULL);
env->ReleaseStringUTFChars(key1,key1Value);
//if(key1Value == NULL)
    //return NULL;

const char *key2Value = (env)->GetStringUTFChars(key2,NULL);

 //if(key2Value == NULL)
     //return NULL;

string key2_str(key2value);
env->ReleaseStringUTFChars(key2,key2Value);


jsize length = env->GetArrayLength(keyTypes);

 if(length == 0)
     return NULL;

// I’m managing the C++ cache from Java side, so getting the
// reference of native cache from Java
jclass CacheService = env->GetObjectClass(thisObject);
Cache* cache= (Cache*) env->GetStaticLongField(CacheService,getCacheField(env,thisObject));
env->DeleteLocalRef(CacheService);
list<int> result;
for(int index=0;index<length;index++){

  jstring keyType =(jstring) env->GetObjectArrayElement(keyTypes,index);
  const char *key_type_str=env->GetStringUTFChars(keyType,NULL);
  string key_type(key_type_str);
  env->ReleaseStringUTFChars(keyType,key_type_str);

   // Cache.cpp code is given below
  list<int> intermediate=cache->get(key2_str+key_type,key1Value);
  result.merge(intermediate);


  intermediate=cache->get(key_type,key1Value);
  result.merge(intermediate);
}

env->ReleaseStringUTFChars(key1,key1Value);

// I would return result list from below code
// For testing I commented all of the above code and
// tested with only below code. I could see Old gen
// Keep growing and GC spikes

    jintArray Ids=env->NewIntArray(50);
          if(Ids == NULL){
              return NULL;
          }
         jint return_Array[50];
         int j=0;
         for(j=0; j<50 ;j++){
            return_Array[j]=j;
          }
          env->SetIntArrayRegion(Ids,0,50,return_Array);
          //env->DeleteLocalRef(Ids);
          return Ids;

and The Cache.cpp code is like below

shared_ptr<multimap<string, SomeObject> > Cache::cacheMap;

list<int>  Cache::get(string key,const char* key1Value){

       SomeObject value;
       list<int> IDs;

       shared_ptr<multimap<string, SomeObject> > readMap=atomic_load(&cacheMap);
       pair<mapIterator,mapIterator> result=readMap->equal_range(key);

       for(auto iterator=result.first; iterator!=result.second; iterator++){
          value=iterator->second;

          if(!(value.checkID(key1Value))){
              IDs.push_front(value.getID(key1Value));

          }
       }

       return IDs;
}

In the code above I guess the Ids array

jintArray Ids=env->NewIntArray(50);

is not getting deleted so I tried to

//env->DeleteLocalRef(Ids);

This doesn't work as the empty array will be returned on Java side.

As described in the code comments, I commented out all the code except for returning a static array to Java. I could see that the Old Generation was getting filled up gradually triggering a Full GC, which I believe is a slow leak problem. I have a slow leak in this cache lookup flow. Am I missing something?

Thanks, Raj

Raj
  • 401
  • 6
  • 20
  • You aren't calling `ReleaseStringUTFChars()` under all possible conditions. – user207421 Oct 09 '19 at 23:41
  • @user207421 there are 3 GetStringUTFChars() for which I have corresponding ReleaseStringUTFChars() too. – Raj Oct 09 '19 at 23:52
  • I repeat. You are not calling it under all possible conditions. Consider the various early returns. – user207421 Oct 09 '19 at 23:55
  • @user207421 Good Point, in my use case I always pass the arguments, so they will never be Null. But will consider checking that too. – Raj Oct 09 '19 at 23:58
  • You need to do more than 'consider' it. You need to *fix* it. It is a memory leak, which is exactly what you are asking about, and it occurs under quite a few conditions, not limited to null arguments. Your code is incorrectly structured. – user207421 Oct 10 '19 at 00:01
  • @user207421 I have updated my code above, and planning to load test it right now. Please let me know if it is correct now. I don't have Nulls so removed that check. – Raj Oct 10 '19 at 00:04
  • @user207421 Still same issue Old generation is growing and then gets GCed. The issue remains the same. Even if I comment out all the code, except for the last part were I just return a Static array of size 50. Still, the old generation keeps growing. – Raj Oct 10 '19 at 01:12
  • Consider switching to `int[] values = new int[]; for(JavaObject o: javaOjctes){ cache.get(o.getkey1,o.getkey2,o.getkeyType[], values); //further processing }` – Alex Cohn Oct 10 '19 at 01:59
  • @AlexCohn Ok will try and let you know – Raj Oct 10 '19 at 02:27
  • 1
    In your updated code you keep using `key1Value` after having called `ReleaseStringUTFChars`, which is incorrect (that pointer is no longer guaranteed to be valid). You also seem to have two calls to `ReleaseStringUTFChars` for that string. – Michael Oct 10 '19 at 06:06
  • @Raj For JNI work, you should be using [RAII](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) techniques so that you are not in the situation where you are not releasing memory when you return from the function. For example, the destructor for an RAII class would automatically call `ReleaseStringUTFChars`, thus a leak would not appear regardless of the reason the function exits (a `return` statement, and exception thrown, etc.) – PaulMcKenzie Oct 10 '19 at 11:24

0 Answers0