[ 
https://issues.apache.org/jira/browse/TRAFODION-2218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15490900#comment-15490900
 ] 

ASF GitHub Bot commented on TRAFODION-2218:
-------------------------------------------

Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/697#discussion_r78787674
  
    --- Diff: core/sql/langman/LmRoutineJavaObj.cpp ---
    @@ -192,6 +192,7 @@ LmResult LmRoutineJavaObj::invokeRoutineMethod(
               (*emitRowPtr_)(NULL,0,&qs);
             }
         }
    +  jni->DeleteLocalRef(jniResult);
    --- End diff --
    
    I have been thinking about this more and talked to @zellerh.
    
    From JNI specs 
http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#global_local
    we have
    "Local References
    **Local references are valid for the duration of a native method call. They 
are freed automatically after the native method returns.** Each local reference 
costs some amount of Java Virtual Machine resource. Programmers need to make 
sure that native methods do not excessively allocate local references. Although 
local references are automatically freed after the native method returns to 
Java, excessive allocation of local references may cause the VM to run out of 
memory during the execution of a native method."
    However in Trafodion's use of JNI, we know by debugging this JIRA that 
localRefs are never freed, unless they are explicitly deleted. This must be 
because the "native" method never returns, as the C++ side inits the JVM. 
Therefore in this use we delete the local Ref to ReturnInfo, but we have new 
local refs to returnedIIL and returnedPIL. Those are deleted later after they 
are used (when this method called next or in the destructor). Those 2 byte 
arrays will not be GCed right after local ref to ReturnInfo is deleted, since 
there is one other local ref pointing to each one of them. They will be GCed 
only when all localRefs pointing to them (including parent class) are deleted. 


> Memory leak from JVM used for TMUDF compile time interaction
> ------------------------------------------------------------
>
>                 Key: TRAFODION-2218
>                 URL: https://issues.apache.org/jira/browse/TRAFODION-2218
>             Project: Apache Trafodion
>          Issue Type: Bug
>          Components: sql-cmp
>    Affects Versions: 2.0-incubating
>            Reporter: Suresh Subbiah
>            Assignee: Suresh Subbiah
>             Fix For: 2.1-incubating
>
>
> JVM heap size for the JNI JVM attached to master executor grows when the 
> executor is used to prepare queries with a TMUDF that have compile time 
> interaction methods defined.
> This can seen with the sessionize_java TMUDF that is part of the regression 
> framework. Please run regress/udr/TEST001 (after commenting out the clean_up 
> command at the end of the test). Then from a new sqlci session do
> set schema sch;
> prepare s from
> select * from udf(sessionize_java(table(select * from t001_Datatypes),
>                                   'C_VARCHAR', 'C_DECIMAL_UNSIGNED', 60));
> -- repeat prepare once more
> -------------- DO heap analysis as shown below
> -- repeat prepare 10 more times
> ------------- DO heap analysis as shown below
> >>>>>>>
> Heap analysis
> -- After 2 prepares
> jmap -histo:live <sqlci-pid> > histo0
> -- After 12 prepares
> jmap -histo:live <sqlci-pid> > histo1
> Second column below is num_occurences and third column is size in bytes
> [ssubbiah@edev08 hive]$ grep ReturnInfo histo* 
> histo0: 221:            12            384  
> org.trafodion.sql.udr.LmUDRObjMethodInvoke$ReturnInfo
> histo1:  94:            72           2304  
> org.trafodion.sql.udr.LmUDRObjMethodInvoke$ReturnInfo
> [ssubbiah@edev08 hive]$ grep "\[B" histo* | grep -v "\[\["
> histo0:   7:         14660        1595584  [B
> histo1:   6:         14714        2302296  [B
> This shows that the nested static class
> .LmUDRObjMethodInvoke$ReturnInfo is leaking. The leak rate is 6 instances per 
> statement compile. This class has byte arrays as data members which are used 
> to return invocation and plan infos. The leak in byte arrays contained is 
> taking more space.
> A leak rate of 14 ReturnInfo instances per compile was seen a different 
> instance. In that case compiling the statement 4000 times would have lead to 
> the 256 MB JVM heap being exhausted. This can result in errors for other 
> users of the JVM such as metadata queries or repository statements.
>  public static class ReturnInfo
>     {
>         // return status:
>         // <0: Internal error, check for Java exception
>         // 0:  Success
>         // >0: User-generated error, returnedSQLState_ and
>         //     returnedErrorMessage_ have details
>         int returnStatus_;
>         String returnedSQLState_;
>         String returnedMessage_;
>         byte [] returnedInvocationInfo_;
>         byte [] returnedPlanInfo_;
> ...
> }
> The fix is remove static keyword here. I could not think of a good reason on 
> why this class needed to be static.I went with the assumption that if 
> possible it is better for a nested class to be non-static as GC cleanup of 
> non-static classes is easier to visualize (same as other non-static classes). 
> Also added a deleteLocalRef where the ReturnInfo is instantiated on the C++ 
> JNI side. It was delete localRef that caused the leak to go away. It is not 
> clear if the change to non-static is necessary. I am going with the 
> assumption that non-static is better here.
> Heap analysis commands used before now show no live instances of ReturnInfo. 
> Byte array size also does not grow with increasing number of compiles.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to