JProxy update by mkitti · Pull Request #112 · JuliaInterop/JavaCall.jl

mkitti · 2020-04-13T17:42:42Z

I have updated the JProxy code from @zot from PR #91 . Tests are now passing and there are no conflicts.

created init_current_vm function

Adding new conversions to convert.jl

array arg conversion did not work for empty arrays adding JConstructor

need to remove references to JavaObject from proxy.jl

aviks · 2020-04-15T20:08:17Z

doc/index.md

+
 ## Major TODOs and Caveats

 * Currently, only a low level interface is available, via `jcall`. As a result, this package is best suited for writing libraries that wrap around existing java packages. Writing user code direcly using this interface might be a bit tedious at present. A high level interface using reflection will eventually be built.


This section of the documentation should probably be rewritten

aviks · 2020-04-15T20:09:41Z

src/JavaCall.jl

       @jimport, jcall, jfield, isnull,
       getname, getclass, listmethods, getreturntype, getparametertypes, classforname,
-       narrow
+       narrow, JProxy, @class, interfacehas, staticproxy


I'm not sure JProxy is the best name here. Might be worth brainstorming a few alternatives

Maybe "JavaObject", because you use it like a Java object?

Yes, that is the most relevant name, the only trouble is that we already have a JavaObject type in JavaCall, so we'll have figure out a transition. However, I think the macro should be called @java.

aviks · 2020-04-15T20:59:53Z

test/runtests.jl

 import Dates
 using Base.GC: gc

+const pxyptr = JavaCall.pxyptr


doesnt seem used below?

Correct, I had used that in a println in an earlier version of the test code and I forgot to remove it

These tests will be moved to a separate sub-package.

aviks · 2020-04-15T21:01:45Z

src/proxy.jl

+
+location(source) = replace(string(source.file), r"^.*/([^/]*)$" => s"\1") * ":" * string(source.line) * ": "
+
+macro verbose(args...)


These seem debug aids. Are they needed now? In general, we should use the Base logging for output.

yes, @verbose is just for debugging the project itself and should eventually be removed or commented out. I left some of the logging commented out so I could uncomment it at need (and I did have to uncomment some of them sometimes :) )

mkitti · 2020-04-27T23:01:41Z

src/convert.jl

-          jstr.ptr, buf)
+          jstr, buf)
    return s
 end


@aviks Could we incorporate the changes to this file convert.jl into JavaCall proper?

Sure. Is the current code likely a bug?

I think it was just quoted out of context. What I meant is that most of the changes to convert.jl seem acceptable to merge. It will have to be updated for the new JNI module though.

mkitti · 2020-04-27T23:07:34Z

src/core.jl

 jprimitive = Union{jboolean, jchar, jshort, jfloat, jdouble, jint, jlong}

-struct JavaMetaClass{T}
+mutable struct JavaMetaClass{T}


Why is it necessary to change this to a mutable struct? I haven't look very hard for the answer, but it might save time if someone could explain it.

Might it make sense to have mutable and unmutable versions of JavaMetaClass?

I'll look around -- I'm pretty sure I had a good reason for that. But maybe not :)

Because the finalizer on line 84 nulls out ptr for safety. If we can guarantee that deleteref will only ever be called from finalize, it can stay immutable I think.

mkitti · 2020-04-27T23:07:43Z

src/jvm.jl

    jvm = unsafe_load(pjvm)
    global jvmfunc = unsafe_load(jvm.JNIInvokeInterface_)
    global jnifunc = unsafe_load(jnienv.JNINativeInterface_) #The JNI Function table
+    initProxy()


We should revert the changes to this file for now.

Looks like init_current_vm() needs initProxy() as well -- unless you're going to remove that call from _init()

mkitti · 2020-04-27T23:10:40Z

test/Test.java

+
+  public String testArrayArgs(Object[] i) {
+    return "java.lang.Object[]";
+  }


The changes to Test.java and the binaries should be reverted for now and moved into a separate sub-package.

mkitti · 2020-05-04T09:31:48Z

src/convert.jl

+convert(::AbstractString, str::Type{JString}) = unsafe_string(str)
 convert(::Type{JString}, str::AbstractString) = JString(str)
 convert(::Type{JObject}, str::AbstractString) = convert(JObject, JString(str))
+convert(::Type{JavaObject{Symbol("java.lang.Double")}}, n::Real) = jnew(Symbol("java.lang.Double"), (jdouble,), Float64(n))
+convert(::Type{JavaObject{Symbol("java.lang.Float")}}, n::Real) = jnew(Symbol("java.lang.Float"), (jfloat,), Float32(n))
+convert(::Type{JavaObject{Symbol("java.lang.Long")}}, n::Real) = jnew(Symbol("java.lang.Long"), (jlong,), Int64(n))
+convert(::Type{JavaObject{Symbol("java.lang.Integer")}}, n::Real) = jnew(Symbol("java.lang.Integer"), (jint,), Int32(n))
+convert(::Type{JavaObject{Symbol("java.lang.Short")}}, n::Real) = jnew(Symbol("java.lang.Short"), (jshort,), Int16(n))
+convert(::Type{JavaObject{Symbol("java.lang.Byte")}}, n::Real) = jnew(Symbol("java.lang.Byte"), (jbyte,), Int8(n))
+convert(::Type{JavaObject{Symbol("java.lang.Character")}}, n::Real) = jnew(Symbol("java.lang.Character"), (jchar,), Char(n))
+convert(::Type{JavaObject{:int}}, n) = convert(jint, n)
+convert(::Type{JavaObject{:long}}, n) = convert(jlong, n)
+convert(::Type{JavaObject{:byte}}, n) = convert(jbyte, n)
+convert(::Type{JavaObject{:boolean}}, n) = convert(jboolean, n)
+convert(::Type{JavaObject{:char}}, n) = convert(jchar, n)
+convert(::Type{JavaObject{:short}}, n) = convert(jshort, n)
+convert(::Type{JavaObject{:float}}, n) = convert(jfloat, n)
+convert(::Type{JavaObject{:double}}, n) = convert(jdouble, n)
+convert(::Type{JavaObject{:void}}, n) = convert(jvoid, n)
+convert(::Type{JavaObject{T}}, ::Nothing) where T = jnull


These convert methods seem fine to merge.

mkitti · 2020-05-04T09:33:23Z

src/convert.jl

 function convert(::Type{JavaObject{T}}, obj::JavaObject{S}) where {T,S}
-    if isConvertible(T, S)   #Safe static cast
-        ptr = ccall(jnifunc.NewLocalRef, Ptr{Nothing}, (Ptr{JNIEnv}, Ptr{Nothing}), penv, obj.ptr)
-        ptr === C_NULL && geterror()
-        return JavaObject{T}(ptr)
+    if isnull(obj)
+        return JavaObject{T}(obj.ptr)
+    elseif isConvertible(T, S)   #Safe static cast
+        return JavaObject{T}(obj.ptr)
    end
-    isnull(obj) && throw(ArgumentError("Cannot convert NULL"))
    realClass = ccall(jnifunc.GetObjectClass, Ptr{Nothing}, (Ptr{JNIEnv}, Ptr{Nothing} ), penv, obj.ptr)
    if isConvertible(T, realClass)  #dynamic cast
-        ptr = ccall(jnifunc.NewLocalRef, Ptr{Nothing}, (Ptr{JNIEnv}, Ptr{Nothing}), penv, obj.ptr)
-        ptr === C_NULL && geterror()
-        return JavaObject{T}(ptr)
+        return JavaObject{T}(obj.ptr)
    end
    throw(JavaCallError("Cannot cast java object from $S to $T"))
 end


This allows for null objects. We could probably merge this.

mkitti · 2020-05-04T09:39:54Z

src/core.jl

 const JString = JavaObject{Symbol("java.lang.String")}
+const jnull = JavaObject(nothing)
+
+JavaObject(ptr::Ptr{Nothing}) = ptr == C_NULL ? JavaObject(ptr) : JavaObject{Symbol(getclassname(getclass(ptr)))}(ptr)


This depends on functions in proxy.jl at the moment, so we will probably move it into proxy.jl

zot · 2020-05-31T09:09:10Z

Folks, do you need any more information from me in order to tie this off?

mkitti · 2020-05-31T15:43:53Z

Hi @zot, thanks for checking in. JavaCall has undergone some major structural changes since a few weeks ago, so this will need to be updated again.

Next on my list is to improve JavaCall's local/global reference management. However, I think the reorganization will make things much more manageable moving forward. For example, all the ccall should be wrapped into the new JNI submodule.

Do we have your permission to continue developing this? My plan remains making this a separate package which has JavaCall as a dependency.

zot · 2020-05-31T20:59:52Z

Do we have your permission to continue developing this? My plan remains making this a separate package which has JavaCall as a dependency.

Of course! JavaCall belongs to you guys -- I wrote the proxy code in order to contribute to the project. Use and modify it as you see fit, I'd just like to see it merged in one form or another...

mkitti · 2020-06-30T06:13:36Z

@zot , @aviks I split the proxy code off into its own JProxies subpackage.

I switched most of the JNI calls over to the JavaCall.JNI module which wraps the ccalls
I disabled most of the memory management code since I want to use JNI.PushLocalFrame and JNI.PopLocalFrame in the future which already does the job of tracking local references and marking them for garbage collection.
All tests in both packages are passing.

The easiest way I found to load JProxies is to symlink the JProxies folder into ~/.julia/dev and then use Pkg.develop. More work is required, but I think this puts us on a more sustainable path.

zot · 2020-06-30T14:02:13Z

JProxies/doc/index.md

+* Create static proxies to access static members
+Primitive types and strings are converted to Julia objects on field accesses and method returns and converted back to Java types when sent as arguments to Java methods.
+*NOTE: Because of this, if you need to call Java methods on a string that you got from Java, you'll have to use `JProxy(str)` to convert the Julia string to a proxied Java string*
+To invoke static methods, set static to true (see below).


I wrote this before I added the @class macro -- I think it should change to this:

To invoke static methods, use @class (see below).

zot · 2020-06-30T14:20:14Z

JProxies/doc/index.md

+julia> JProxy(@jimport(java.lang.System)).getName()
+"java.lang.System"
+
+julia> JProxy(@jimport(java.lang.System);static=true).out.println("hello")


I'd change this to
julia> @class(java.lang.System).out.println("hello")

aviks · 2020-07-01T22:09:04Z

Can we just make this a submodule for the JavaCall module, so doing using JavaCall; using JavaCall.JProxy gets you the new behaviour?

mkitti · 2020-07-02T00:11:04Z

Can we just make this a submodule for the JavaCall module, so doing using JavaCall; using JavaCall.JProxy gets you the new behaviour?

We could do that although the direction I was heading in was to spin it out so that it can live as its own package. Currently, you could import JProxies without explicitly importing JavaCall.

using JProxies
JProxies.init()

Under the hood that will still use JavaCall. You made a point once that there was a lot of the proxy code to support which could make future maintenance more difficult. Dividing the code clearly like this is beneficial for maintainability I think.

There is still some code I actually want to move into JavaCall itself. For example, listfields probably belongs in reflect.jl.

zot · 2020-07-05T03:58:08Z

Can we just make this a submodule for the JavaCall module, so doing using JavaCall; using JavaCall.JProxy gets you the new behaviour?

We could do that although the direction I was heading in was to spin it out so that it can live as its own package. Currently, you could import JProxies without explicitly importing JavaCall.
using JProxies
JProxies.init()
Under the hood that will still use JavaCall. You made a point once that there was a lot of the proxy code to support which could make future maintenance more difficult. Dividing the code clearly like this is beneficial for maintainability I think.

That sounds like a good direction to me. That way people who just want to use JavaCall don't need to load in JProxy and people who want JProxy don't need to worry about JavaCall.

There is still some code I actually want to move into JavaCall itself. For example, listfields probably belongs in reflect.jl.

Yes, I wrote some useful utility code that could move into JavaCall but I wanted to keep my changes to the base package to a minimum so I kept as much as I could in proxy.jl.

zot · 2020-11-01T18:27:50Z

Is this dying on the vine?

mkitti · 2020-11-02T02:06:27Z

Most of the changes to core JavaCall were merged in b5692cc a few months ago.

We're preparing for a JavaCall 0.8 release. Taro just set compatibility with 0.8.

It looks like we have conflicts to fix here. The jnull thing should be tested again.

mkitti · 2020-11-02T09:16:37Z

Add a JProxy subpackage

Bill Burdick and others added 30 commits September 8, 2018 15:28

allow initializing from running VM to allow call-in/call-out

28fa72c

created init_current_vm function

untabified example code

d5cab17

Added usage section to doc/index.md

1bf2c99

slight wording improvement

0b63144

checkpoint proxy work

d2d6a3d

cleanup and more conversions

c92d06d

Adding new conversions to convert.jl

JProxy for better call-in to Java

dc06735

better type conversion from Java to Julia

704221e

fixing tests to conform to new JProxy "static" keyword

9f1154b

Merge branch 'master-base'

b1732e2

adding documentation

6e60b35

associate Java null with Julia nothing

6cd466d

more docu

03089ab

cleaning up types, removing redundant fields, etc.

53a49c3

cleaned up typeInf macro

1dd74b7

fixing arg conversion bug, changing proxy to use generated methods

459fc57

array arg conversion did not work for empty arrays adding JConstructor

checkpoint

fed1446

checkpoint

f1ea8af

improved design, need to eliminate JProxy's references to JavaObject

60f328c

improving value conversion to prepare to handle array parameters

cfb81a3

switching methods to use generated arg types

2dffa48

localrefs for convert_arg, code cleanup, interfaces

97a8bc8

local ref management, PtrBox, interfaces

22cab6f

Basic version that seems to work but needs much improvement

d84c9d2

need to remove references to JavaObject from proxy.jl

Boxing still not working

590ed82

All tests work (including boxing)

7d84f82

Removed GC from tests

6267c07

removed GC from tests

0e6cbbf

Added test for interface argument, a.addAll(b)

2354c5a

getting array argument conversion working, fixing bugs

f577272

aviks reviewed Apr 21, 2020

View reviewed changes

mkitti commented Apr 27, 2020

View reviewed changes

Merge branch 'master' into zot

416690a

mkitti commented May 4, 2020

View reviewed changes

Test with new appveyor.yml for x86

13aa29d

mkitti mentioned this pull request Jun 18, 2020

Create JavaRef, JavaLocalRef, and JavaGlobalRef to hold references and manage memory #124

Merged

Split JProxies into a separate package, merge javaref branch

57aead9

zot reviewed Jun 30, 2020

View reviewed changes

mkitti added 4 commits July 13, 2020 22:35

Expose @Class macro for static method references of classes

001d4ee

Transfer listfields from JProxies to JavaCall

9801641

Merge branch 'master' into zot

67f0f2d

Merge branch 'master' into zot

238ea43

Merge branch 'master' into zot

e72872a

mkitti merged commit 4625a51 into JuliaInterop:master Nov 2, 2020


		## Major TODOs and Caveats

		* Currently, only a low level interface is available, via `jcall`. As a result, this package is best suited for writing libraries that wrap around existing java packages. Writing user code direcly using this interface might be a bit tedious at present. A high level interface using reflection will eventually be built.


		location(source) = replace(string(source.file), r"^./([^/])$" => s"\1") * ":" * string(source.line) * ": "

		macro verbose(args...)

Conversation

mkitti commented Apr 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zot Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zot Jun 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zot commented May 31, 2020

Uh oh!

mkitti commented May 31, 2020

Uh oh!

zot commented May 31, 2020

Uh oh!

mkitti commented Jun 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aviks commented Jul 1, 2020

Uh oh!

mkitti commented Jul 2, 2020

Uh oh!

zot commented Jul 5, 2020

Uh oh!

zot commented Nov 1, 2020

Uh oh!

mkitti commented Nov 2, 2020

Uh oh!

mkitti commented Nov 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

zot Apr 28, 2020 •

edited

Loading

zot Jun 18, 2020 •

edited

Loading