Skip to content

Commit

Permalink
[jnienv-gen] Explore additional invocation strategies.
Browse files Browse the repository at this point in the history
@migueldeicaza suggested a strategy for speeding up JNI invocations:
Bundle the exception check with the method invocation.

The logic here is that JNIEnv::ExceptionOccurred() should be a *fast*
invocation -- it only (currently) returns a field from the JNIEnv*
struct. No GC, no stopping the world, ~no overhead to speak of.

Yet most of the "important" JNI invocations *always* need to call it,
and because of how we designed things this invocation is through a
delegate:

	var tmp = JniEnvironment.Current.Invoker.CallObjectMethod (JniEnvironment.Current.EnvironmentPointer, @object.SafeHandle, method.ID);

	var __e - JniEnvironment.Exceptions.ExceptionOccurred();
	if (__e.IsValid /* or whatever... */) {
		JniEnvironment.Exceptions.ExceptionClear();
		throw WrapThatException (__e);
	}

	return tmp;

JNI ~requires that you do these two JNI invocations (or the exception
will be left pending, and re-raised the next time you call into Java,
and possibly abort if *another* exception is then thrown...).
In the case of Xamarin.Android and the Java.Interop "SafeHandle" and
"IntPtrs" backends (commit 9b85e8d), this results in a second
delegate invocation and corresponding overhead.

@migueldeicaza asked: could we instead call
JNIEnv::ExceptionOccurred() from a C wrapper as part of the "source"
JNIEnv function pointer invocation, performing two JNI calls from
a custom C function (as opposed to one JNI call per C wrapper, as done
in commit 802842a).

The C# binding thus becomes:

	// C#
	IntPtr thrown;
	var tmp = JavaInterop_CallObjectMethod (JniEnvironment.Current.EnvironmentPointer, out thrown, @object.SafeHandle, method.ID);

	if (thrown != IntPtr.Zero) {
		JniEnvironment.Exceptions.ExceptionClear();
		throw WrapThatException (thrown);
	}

	return tmp;

While the C wrapper is:

	/* C */
	jobject
	JavaInterop_CallObjectMethod (JNIEnv *env, jthrowable *_thrown, jobject object, jmethodID method)
	{
		*_thrown = NULL;
		jobject _r_ = (*env)->CallObjectMethod (env, object, method);
		*_thrown = (*env)->ExceptionOccurred (env);
		return _r_;
	}

The answer: Yes, we can, and it saves ~9% vs. the IntPtr backend.

Update jnienv-gen to emit this third backend alongside the previous
backends, and (for good measure?) emit a *fourth* Xamarin.Android-like
backend for easier/saner comparisions.

The tests/invocation-overhead results:

	# SafeTiming timing: 00:00:08.2506653
	#	Average Invocation: 0.00082506653ms
	# JIIntPtrTiming timing: 00:00:02.4018293
	#	Average Invocation: 0.00024018293ms
	# JIPinvokeTiming timing: 00:00:02.2677633
	#	Average Invocation: 0.00022677633ms
	# XAIntPtrTiming timing: 00:00:02.3472511
	#	Average Invocation: 0.00023472511ms

SafeTiming is the SafeHandle backend; as noted in commit 25de1f3, the
performance is attrocious.

JIIntPtrTiming is JniObjectReference + IntPtrs (9b85e8d).

JIPinvokeTiming is @migueldeicaza's suggestion, outlined above.

XAIntPtrTiming is a Xamarin.Android-like "all IntPtrs all the time!"
backend, for comparison purposes.

Of note is that JIIntPtrTiming -- what I was planning on using for a
future Xamarin.Android integration (commit 25de1f3) -- is ~1-2%
slower than XAIntPtrTiming. Not great, but acceptable.

Migrating to @migueldeicaza's P/Invoke approach results in performance
that's 3-4% *faster* than Xamarin.Android, and ~5-9% faster than
JIIntPtrTiming.

Which means the Xamarin.Android integration should use the P/Invoke
implementation strategy.
  • Loading branch information
jonpryor committed Oct 23, 2015
1 parent 8c52dbd commit 926e4bc
Show file tree
Hide file tree
Showing 11 changed files with 12,520 additions and 13,965 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
bin
obj
LocalJDK
TestResult.xml
25 changes: 23 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,30 @@ clean:
$(XBUILD) /t:Clean
rm -Rf bin/$(CONFIGURATION)

bin/$(CONFIGURATION)/libNativeTiming.dylib: tests/NativeTiming/timing.c
JDK = JavaDeveloper.pkg
JDK_URL = http://adcdownload.apple.com/Developer_Tools/java_for_os_x_2013005_developer_package/java_for_os_x_2013005_dp__11m4609.dmg

LOCAL_JDK_HEADERS = LocalJDK/System/Library/Frameworks/JavaVM.framework/Versions/A/Headers

osx-setup: $(LOCAL_JDK_HEADERS)/jni.h

$(LOCAL_JDK_HEADERS)/jni.h:
@if [ ! -f $(JDK) ]; then \
echo "Please download '$(JDK)', from: $(JDK_URL)" ; \
exit 1; \
fi
-mkdir LocalJDK
_jdk="$$(cd `dirname "$(JDK)"`; pwd)/`basename "$(JDK)"`" ; \
(cd LocalJDK; xar -xf $$_jdk)
(cd LocalJDK; gunzip -c JavaEssentialsDev.pkg/Payload | cpio -i)

bin/$(CONFIGURATION)/libNativeTiming.dylib: tests/NativeTiming/timing.c $(LOCAL_JDK_HEADERS)/jni.h
mkdir -p `dirname "$@"`
gcc -g -shared -o $@ $< -m32 -I $(LOCAL_JDK_HEADERS)

bin/$(CONFIGURATION)/libJavaInterop.dylib: JniEnvironment.g.c $(LOCAL_JDK_HEADERS)/jni.h
mkdir -p `dirname "$@"`
gcc -g -shared -o $@ $< -m32 -I /System/Library/Frameworks/JavaVM.framework/Headers
gcc -g -shared -o $@ $< -m32 -I $(LOCAL_JDK_HEADERS)

bin/$(CONFIGURATION)/Java.Interop-Tests.dll: $(wildcard src/Java.Interop/*/*.cs src/Java.Interop/Tests/*/*.cs)
$(XBUILD)
Expand Down
24 changes: 8 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,17 @@ At the time of this writing, this links to

[osx-jdk6]: http://adcdownload.apple.com/Developer_Tools/java_for_os_x_2013005_developer_package/java_for_os_x_2013005_dp__11m4609.dmg

*Furthermore*, if running on Yosemite you must *also* download the latest
[Java for OS X package](http://support.apple.com/downloads/#java), currently
[JavaForOSX2014-001.dmg](http://support.apple.com/downloads/DL1572/en_US/JavaForOSX2014-001.dmg).
Unfortunately, you can't *install* it on El Capitan. It'll install...but it
won't *do* anything, probably because of [System Integrity Protection][sip].

Once download, you need to "remove" any previously installed Java packages, as
the Developer package won't install over a newer runtime packages:
[sip]: https://en.wikipedia.org/wiki/System_Integrity_Protection

sudo mv /System/Library/Frameworks/JavaVM.framework /System/Library/Frameworks/JavaVM.framework-Yosemite
To develop on El Capitan, download the above
`java_for_os_x_2013005_dp__11m4609.dmg` file, open it within Finder,
copy the contained `JavaDeveloper.pkg` file into this directory,
then run the `osx-setup` target:

Then install the Developer Package `java_for_os_x_2013005_dp__11m4609.dmg`,
then install the runtime package `JavaForOSX2014-001.dmg`.

If you fail to re-install the runtime package, then `jar` will fail to run:

$ jar cf "../../bin/Debug/java-interop.jar" -C "../../bin/Debug/ji-classes" .
java.lang.AssertionError: Platform not recognized
at sun.nio.fs.DefaultFileSystemProvider.create(DefaultFileSystemProvider.java:73)
at java.nio.file.FileSystems$DefaultFileSystemHolder.getDefaultProvider(FileSystems.java:108)
...
$ make osx-setup JDK=JavaDeveloper.pkg


## Type Safety
Expand Down
11 changes: 11 additions & 0 deletions src/Java.Interop/Java.Interop/JniEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,17 @@ public Exception GetExceptionForLastThrowable ()
JniEnvironment.Current.LogCreateLocalRef (e);
return JavaVM.GetExceptionForThrowable (ref e, JniObjectReferenceOptions.DisposeSourceReference);
}

public Exception GetExceptionForLastThrowable (IntPtr thrown)
{
if (thrown == IntPtr.Zero)
return null;
var e = new JniObjectReference (thrown, JniObjectReferenceType.Local);
// JniEnvironment.Errors.ExceptionDescribe ();
JniEnvironment.Exceptions.ExceptionClear ();
JniEnvironment.Current.LogCreateLocalRef (e);
return JavaVM.GetExceptionForThrowable (ref e, JniObjectReferenceOptions.DisposeSourceReference);
}
}
}

24 changes: 20 additions & 4 deletions tests/invocation-overhead/Makefile
Original file line number Diff line number Diff line change
@@ -1,10 +1,26 @@
all: test-overheads.exe test-overheads-xa.exe
JNIENV_GEN = ../../bin/Debug/jnienv-gen.exe
LOCAL_JDK_HEADERS = ../../LocalJDK/System/Library/Frameworks/JavaVM.framework/Versions/A/Headers

all: test-overheads.exe libJavaInterop.dylib

clean:
-rm test-overheads.exe test-overheads.exe.mdb
-rm -Rf libJavaInterop.dylib*

HANDLE_FEATURES = \
-d:FEATURE_HANDLES_ARE_SAFE_HANDLES \
-d:FEATURE_HANDLES_ARE_INTPTRS \
-d:FEATURE_HANDLES_ARE_INTPTRS_WITH_PINVOKES_EX \
-d:FEATURE_HANDLES_ARE_XA_INTPTRS

test-overheads.exe: test-overheads.cs jni.cs
mcs -out:$@ -unsafe -d:FEATURE_HANDLES_ARE_SAFE_HANDLES -d:FEATURE_HANDLES_ARE_INTPTRS $^
mcs -out:$@ -unsafe $(HANDLE_FEATURES) $^

jni.c jni.cs: $(JNIENV_GEN)
mono $< jni.cs jni.c

test-overheads-xa.exe: test-overheads.cs jni-xa.cs
mcs -out:$@ -unsafe -d:XA -d:FEATURE_HANDLES_ARE_SAFE_HANDLES -d:FEATURE_HANDLES_ARE_INTPTRS $^
libJavaInterop.dylib: jni.c
gcc -g -shared -o $@ $< -m32 -I $(LOCAL_JDK_HEADERS)

run:
mono --debug test-overheads.exe
Loading

0 comments on commit 926e4bc

Please sign in to comment.