Review request for 7131793: [macosx] some cleanup in OGL pipeline code (original) (raw)

Dmitry Cherepanov dmitry.cherepanov at oracle.com
Tue Jan 31 03:36:30 PST 2012

Mike Swingler wrote:

On Jan 30, 2012, at 5:59 AM, Dmitry Cherepanov wrote:

Mike Swingler wrote:

On Jan 27, 2012, at 7:17 AM, Dmitry Cherepanov wrote:

Hello, Here's some changes to remove some stale code from the OGL pipeline. This code has been introduced as a part of the initial implementation for CALayer-based rendering and disabled by the changes for MACOSXPORT-766. http://cr.openjdk.java.net/~dcherepanov/7131793/webrev.1/ <http://cr.openjdk.java.net/%7Edcherepanov/7131793/webrev.1/> One tiny detail: I'd recommend using -jObjectWithEnv: instead of -jObject here (because it's a little faster to not re-fetch the env): + JNFCallVoidMethod(env, [self.javaLayer jObject], jmdrawInCGLContext); On second thought, seems like -jObject simply returns the global reference that the wrapper holds without obtaining the env. What's the reason for re-fetching the env? That is correct...we are returning the global env, I had forgot about that. It doesn't seem safe though, if the wrapper gets dealloc'd while the ref is still held in a local. :-/ As is though, I don't think we can change the API at this point. Thanks for the clarification.

Mike Swingler wrote:

On Jan 30, 2012, at 1:52 AM, Sergey Bylokhov wrote:

124JNFCallVoidMethod(env, [self.javaLayer jObjectWithEnv:env], jmdrawInCGLContext);

Should we delete a local reference which returned from [self.javaLayer jObjectWithEnv:env]? And check it for null? JNFJObjectWrapper.h ...... - (jobject) jObjectWithEnv:(JNIEnv *)env; // returns a new local-ref, must be released with DeleteLocalRef ...... I believe local refs are collected after the scope of the local function returns, so I don't believe it's strictly necessary. Is that right? Seems like it's better to explicitly free local references in native callbacks. The doc in http://java.sun.com/docs/books/jni/html/refs.html#27567 ("Freeing Local References" section) describes:

"Your native method does not return at all. For example, a native method may enter an infinite event dispatch loop. It is crucial to release local references created inside the loop so that they do not accumulate indefinitely, resulting in a memory leak."

Here's the new webrev (with explicit releasing applied) - http://cr.openjdk.java.net/~dcherepanov/7131793/webrev.3/

Thanks, Dmitry

More information about the macosx-port-dev mailing list