From 06cfd8383e2701d5cb58ac1d7e132d0a9d4bf5d0 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 9 Jan 2015 16:22:54 -0500 Subject: [PATCH] [Java.Interop.Export] Use JniEnvironment in marshal methods. As noted in commit 2f9fece8, Java.Interop.Export.ExportedMemberBuilder doesn't need to use JniEnvironment within it's generated code, as JniType.RegisterNativeMethods() automagically wraps delegates with the required JniEnvironment usage code. Thus, ExportedMemberBuilder doesn't *need* to emit JniEnvironment use RIGHT NOW. The question is rather regarding the future: one of the performance issues that needs investigation on Xamarin.Android is to improve support for AOT, so that startup overhead can be reduced/minimized by pre-JITing as much as possible via AOT. The problem with providing AOT support is that Xamarin.Android -- just like Java.Interop (unsurprisingly) -- ALSO automagically wraps all methods registered with the JVM with System.Reflection.Emit-generated code at runtime. It is thus not possible to AOT *everything*. *Lots* of things can be AOT'd, just not everything, and this is something that we would like to address, meaning all required sources of runtime code generation need to be removable. With that in mind, long-term we don't want ExportedMemberBuilder to rely on JniType.RegisterNativeMethods() behavior. (In fact, we'd want to REMOVE the currently required wrapping behavior from JniType.RegisterNativeMethods()!) Instead, long-term it would be "interesting" if we could use ExportedMemberBuilder to process assemblies, generate the JNI method marshaling code for ~everything, store that into a NEW assembly (as part of the build process), and AOT the marshaling assembly! How cool would that be? :-D But to even get there, we need the marshal code to be self-contained, and not rely upon runtime code generation semantics from "elsewhere". --- .../Java.Interop/ExportedMemberBuilder.cs | 53 ++++++- .../Java.Interop/ExportedMemberBuilderTest.cs | 130 ++++++++++++++---- 2 files changed, 150 insertions(+), 33 deletions(-) diff --git a/src/Java.Interop.Export/Java.Interop/ExportedMemberBuilder.cs b/src/Java.Interop.Export/Java.Interop/ExportedMemberBuilder.cs index 461aeb7b2..6891fd6c0 100644 --- a/src/Java.Interop.Export/Java.Interop/ExportedMemberBuilder.cs +++ b/src/Java.Interop.Export/Java.Interop/ExportedMemberBuilder.cs @@ -108,6 +108,15 @@ public virtual LambdaExpression CreateMarshalFromJniMethodExpression (ExportAttr var jnienv = Expression.Parameter (typeof (IntPtr), "__jnienv"); var context = Expression.Parameter (typeof (IntPtr), "__context"); + var envp = Expression.Variable (typeof (JniEnvironment), "__envp"); + var envpVars = new List () { + envp, + }; + + var envpBody = new List () { + Expression.Assign (envp, CreateJniEnvironment (jnienv)), + }; + var jvm = Expression.Variable (typeof (JavaVM), "__jvm"); var variables = new List () { jvm, @@ -152,12 +161,17 @@ public virtual LambdaExpression CreateMarshalFromJniMethodExpression (ExportAttr ParameterExpression ret = null; if (method.ReturnType == typeof (void)) { marshalBody.Add (invoke); + envpBody.Add ( + Expression.TryCatchFinally ( + Expression.Block (variables, marshalBody), + CreateDisposeJniEnvironment (envp), + CreateMarshalException (envp, null))); } else { var jniRType = GetMarshalToJniReturnType (method.ReturnType); var exit = Expression.Label (jniRType, "__exit"); ret = Expression.Variable (jniRType, "__jret"); var mret = Expression.Variable (method.ReturnType, "__mret"); - variables.Add (ret); + envpVars.Add (ret); variables.Add (mret); marshalBody.Add (Expression.Assign (mret, invoke)); if (jniRType == method.ReturnType) @@ -170,9 +184,15 @@ public virtual LambdaExpression CreateMarshalFromJniMethodExpression (ExportAttr marshalBody.Add (Expression.Assign (ret, marshalExpr)); } marshalBody.Add (Expression.Return (exit, ret)); - marshalBody.Add (Expression.Label (exit, ret)); - } + envpBody.Add ( + Expression.TryCatchFinally ( + Expression.Block (variables, marshalBody), + CreateDisposeJniEnvironment (envp), + CreateMarshalException (envp, exit))); + + envpBody.Add (Expression.Label (exit, Expression.Default (jniRType))); + } var funcTypeParams = new List () { typeof (IntPtr), @@ -188,7 +208,7 @@ public virtual LambdaExpression CreateMarshalFromJniMethodExpression (ExportAttr var bodyParams = new List { jnienv, context }; bodyParams.AddRange (marshalParameters); - var body = Expression.Block (variables, marshalBody); + var body = Expression.Block (envpVars, envpBody); return Expression.Lambda (marshalerType, body, bodyParams); } @@ -271,6 +291,31 @@ static Func F (Func func) return func; } + static Expression CreateJniEnvironment (ParameterExpression jnienv) + { + return Expression.New ( + typeof (JniEnvironment).GetConstructor (new []{typeof (IntPtr)}), + jnienv); + } + + static CatchBlock CreateMarshalException (ParameterExpression envp, LabelTarget exit) + { + var spe = typeof (JniEnvironment).GetMethod ("SetPendingException"); + var ex = Expression.Variable (typeof (Exception), "__e"); + var body = new List () { + Expression.Call (envp, spe, ex), + }; + if (exit != null) { + body.Add (Expression.Return (exit, Expression.Default (exit.Type))); + } + return Expression.Catch (ex, Expression.Block (body)); + } + + static Expression CreateDisposeJniEnvironment (ParameterExpression envp) + { + return Expression.Call (envp, typeof (JniEnvironment).GetMethod ("Dispose")); + } + static Expression GetThis (Expression vm, Type targetType, Expression context) { return Expression.Call ( diff --git a/src/Java.Interop.Export/Tests/Java.Interop/ExportedMemberBuilderTest.cs b/src/Java.Interop.Export/Tests/Java.Interop/ExportedMemberBuilderTest.cs index 1173dfd0c..8e07a16a6 100644 --- a/src/Java.Interop.Export/Tests/Java.Interop/ExportedMemberBuilderTest.cs +++ b/src/Java.Interop.Export/Tests/Java.Interop/ExportedMemberBuilderTest.cs @@ -167,12 +167,26 @@ public void CreateMarshalFromJniMethodExpression_InstanceAction () CheckCreateInvocationExpression (null, t, m, typeof (Action), @"void (IntPtr __jnienv, IntPtr __context) { - JavaVM __jvm; - ExportTest __this; + JniEnvironment __envp; - __jvm = JniEnvironment.Current.JavaVM; - __this = __jvm.GetObject(__context); - __this.InstanceAction(); + __envp = new JniEnvironment(__jnienv); + try + { + JavaVM __jvm; + ExportTest __this; + + __jvm = JniEnvironment.Current.JavaVM; + __this = __jvm.GetObject(__context); + __this.InstanceAction(); + } + catch (Exception __e) + { + __envp.SetPendingException(__e); + } + finally + { + __envp.Dispose(); + } }"); } @@ -209,10 +223,24 @@ public void CreateMarshalFromJniMethodExpression_StaticAction () CheckCreateInvocationExpression (null, t, a.Method, typeof(Action), @"void (IntPtr __jnienv, IntPtr __context) { - JavaVM __jvm; + JniEnvironment __envp; + + __envp = new JniEnvironment(__jnienv); + try + { + JavaVM __jvm; - __jvm = JniEnvironment.Current.JavaVM; - ExportTest.StaticAction(); + __jvm = JniEnvironment.Current.JavaVM; + ExportTest.StaticAction(); + } + catch (Exception __e) + { + __envp.SetPendingException(__e); + } + finally + { + __envp.Dispose(); + } }"); } @@ -227,12 +255,26 @@ public void CreateMarshalFromJniMethodExpression_StaticActionInt32String () CheckCreateInvocationExpression (e, t, m.Method, typeof (Action), @"void (IntPtr __jnienv, IntPtr __context, int i, IntPtr native_v) { - JavaVM __jvm; - string v; + JniEnvironment __envp; + + __envp = new JniEnvironment(__jnienv); + try + { + JavaVM __jvm; + string v; - __jvm = JniEnvironment.Current.JavaVM; - v = Strings.ToString(native_v); - ExportTest.StaticActionInt32String(i, v); + __jvm = JniEnvironment.Current.JavaVM; + v = Strings.ToString(native_v); + ExportTest.StaticActionInt32String(i, v); + } + catch (Exception __e) + { + __envp.SetPendingException(__e); + } + finally + { + __envp.Dispose(); + } }"); } @@ -247,16 +289,31 @@ public void CreateMarshalFromJniMethodExpression_FuncInt64 () CheckCreateInvocationExpression (e, t, m, typeof (Func), @"long (IntPtr __jnienv, IntPtr __context) { - JavaVM __jvm; - ExportTest __this; + JniEnvironment __envp; long __jret; - long __mret; - __jvm = JniEnvironment.Current.JavaVM; - __this = __jvm.GetObject(__context); - __mret = __this.FuncInt64(); - __jret = __mret; - return __jret; + __envp = new JniEnvironment(__jnienv); + try + { + JavaVM __jvm; + ExportTest __this; + long __mret; + + __jvm = JniEnvironment.Current.JavaVM; + __this = __jvm.GetObject(__context); + __mret = __this.FuncInt64(); + __jret = __mret; + return __jret; + } + catch (Exception __e) + { + __envp.SetPendingException(__e); + return default(long); + } + finally + { + __envp.Dispose(); + } }"); } @@ -271,16 +328,31 @@ public void CreateMarshalFromJniMethodExpression_FuncIJavaObject () CheckCreateInvocationExpression (e, t, m, typeof (Func), @"IntPtr (IntPtr __jnienv, IntPtr __context) { - JavaVM __jvm; - ExportTest __this; + JniEnvironment __envp; IntPtr __jret; - JavaObject __mret; - __jvm = JniEnvironment.Current.JavaVM; - __this = __jvm.GetObject(__context); - __mret = __this.FuncIJavaObject(); - __jret = Handles.NewReturnToJniRef(__mret); - return __jret; + __envp = new JniEnvironment(__jnienv); + try + { + JavaVM __jvm; + ExportTest __this; + JavaObject __mret; + + __jvm = JniEnvironment.Current.JavaVM; + __this = __jvm.GetObject(__context); + __mret = __this.FuncIJavaObject(); + __jret = Handles.NewReturnToJniRef(__mret); + return __jret; + } + catch (Exception __e) + { + __envp.SetPendingException(__e); + return default(IntPtr); + } + finally + { + __envp.Dispose(); + } }"); } }