Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JClass is not properly supported #11

Closed
dfdx opened this issue Jul 3, 2015 · 14 comments
Closed

JClass is not properly supported #11

dfdx opened this issue Jul 3, 2015 · 14 comments

Comments

@dfdx
Copy link
Collaborator

dfdx commented Jul 3, 2015

Although in Java Class is, well, a class itself, in JavaCall it is not an instance of JavaObject. This leads to errors in some cases. Here's an example.

First, let's call .toString() method of some object to make sure we do everything right:

julia> using JavaCall
Loaded /usr/lib/jvm/java-7-openjdk-amd64/jre/lib/amd64/server/libjvm.so

julia> JavaCall.init([])

julia> jcall(JString("test"), "toString", JString, ())
"test"

And now let's do the same thing with .getClass which returns JClass:

julia> jcall(JString("test"), "getClass", JClass, ())
ERROR: `write` has no method matching write(::IOBuffer, ::Nothing)
 in write at io.jl:32
 in method_signature at /home/<username>/.julia/v0.3/JavaCall/src/JavaCall.jl:447
 in jcall at /home/<username>/.julia/v0.3/JavaCall/src/JavaCall.jl:271

The error comes from signature() method which is supposed to return single-character descriptor of a type, but, when given JClass, returns nothing:

julia> JavaCall.signature(JString)
"Ljava/lang/String;"

julia> JavaCall.signature(JClass)

julia>

Most probably, for this specific case it's possible to simply overload signature() method, but I'm not sure there will be no other issues with distinction between JClass / JavaObject, so I leave this design question to you.

@dfdx
Copy link
Collaborator Author

dfdx commented Jul 4, 2015

More generally, I'd say it's a question of supporting generic classes, since JClass{T} can be represented as JavaObject{symbol(java.lang.class)} additionally parameterized by T (where T is a name of another Java class). Working on Julia-Spark interface I find it pretty helpful to be able to handle generic collections (e.g. RDD<String> or even simpler Iterator<String>).

There are 2 ways to implement it, though. On one hand, we can automatically convert known types back and forward using information about metaclass similar to how it's done for arrays now. On another hand, we can avoid conversions and work with general JavaObjects (without dispatching on specific Java type). We can, though, keep type specification as a field, though, just for user's reference. E.g. something like this:

type JavaObject
    ptr::Ptr{Void}
    class::String
end

User would have to convert data between Julia and Java types himself, but general interface would be much simpler (imagine automatic conversion for something like Iterator<List<byte[]>>).

Personally I tend to the second option because of JVM's type erasure: for runtime there's only primitive types and Object, so following the same model should be easier and more robust in terms of JNI communication.

@aviks
Copy link
Collaborator

aviks commented Jul 6, 2015

Yes, this is currently not supported, but is an interesting problem.. I think the best way to design this would be to look at actual use cases.

Therefore, I would be interested in how you intend on using, eg, RDD<String> or Iterator<String> etc. As you say, the type parameters are erased at runtime. So what information about Java type parameters would available and/or useful on the Julia side? Some (theoretical) example code would be useful.

@dfdx
Copy link
Collaborator Author

dfdx commented Jul 6, 2015

One use case I have in mind is automatic data conversion and serialization. In Julia-Spark connector, for example, it would be nice to have JJavaRDD{String} instead of generic JJavaRDD, and be able to automatically convert it to Julia's RDD{String} without the need for a user to specify converter.

Generic type should not be necessarily expressed as Julia's type parameter, though. In fact, I'm currently experimenting with an approach where Java's class is not a part of JavaObject signature, but instead just a field. This way we can follow JVM's internals as close as possible (working with general-purpose Objects), but also keep meta information close to objects themselves. I will submit issue/pull request for discussion in a couple of days to better express my idea.

@aviks
Copy link
Collaborator

aviks commented Jul 7, 2015

The advantage of keeping the Java type as part of the julia type is that you can dispatch based on the Java type, without having to create a julia wrapper type for each java type. That is a huge benefit for wrapping api's, and I would be loath to give that away.

I do want to support your use case, but don't understand the details of your usecase yet. So my confusion is as follows. If you have a parameterised java method that returns, say, JJavaRDD{T} , then, when you call that method at runtime, there is no information on what that T can be. You will have to do a runtime check and cast based on external information. So the question is, at what point do you know that T=java.lang.String , and where can you store the information.

@dfdx
Copy link
Collaborator Author

dfdx commented Jul 7, 2015

Sure, I'm not talking about inferring type parameters from JVM, but instead about attaching it to JavaObject. Anyway, I think we should consider supporting generic types, but implementation details will depend on many other questions. Right now I'm interested in resolving issue with JClass, and after inspecting source code more closely I don't think generics is a good approach.

The advantage of keeping the Java type as part of the julia type is that you can dispatch based on the Java type, without having to create a julia wrapper type for each java type.

Do you mean dispatching in user's code? Something like this:

foo(obj::JavaObject{"A"}) = print("this is A")
foo(obj::JavaObject{"B"}) = print("this is B")

?

@dfdx
Copy link
Collaborator Author

dfdx commented Jul 7, 2015

Ok, after long-long research I see that this issue is nothing more than naming confusion. JClass is very different from, say, JString, and shouldn't be used in jcall at all. Instead, java.lang.Class should be exported. Following code works:

julia> using JavaCall
Loaded /usr/lib/jvm/java-7-openjdk-amd64/jre/lib/amd64/server/libjvm.so

julia> JavaCall.init([])

julia> JClassClass = @jimport java.lang.Class
JavaObject{symbol("java.lang.Class")} (constructor with 2 methods)

julia> jcall(JString("test"), "getClass", JClassClass, ())
JavaObject{symbol("java.lang.Class")}(Ptr{Void} @0x00000000060f84f0)

Does it make sense to rename JClass to something like JavaClass to avoid confusion?

@dfdx
Copy link
Collaborator Author

dfdx commented Jul 7, 2015

BTW, don't you mind if I split code into several files and adjust indents according to CONTRIBUTING.md? If it's not a part of your style, such transformation should make code more easily readable and maintainable.

@aviks
Copy link
Collaborator

aviks commented Jul 7, 2015

Ok, after long-long research I see that this issue is nothing more than naming confusion ...

Exactly. After I wrote the last comment, and went to bed, that is what i was thinking: "For method return values of java.lang.Class, JavaObject{:java.lang.Class} should be used for. Maybe that is all that is required. I'll write a comment in the morning" :) Sorry for not picking that up earlier.

Does it make sense to rename JClass to something like JavaClass to avoid confusion

Probably. As you know, java.lang.Class{T} serves two purposes: one, as a description of the type of the object, and two as a collection of methods to operate on that type. The JVM manages the duality. However, internally these functions are different, as shown by the JNI API which returns different pointers for the two (hence the need for a separate JClass). I find it useful to compare this to the metaclass or eigen-class concepts in Ruby.

BTW, don't you mind if I split code into several files and adjust indents

I personally find it simpler to keep it all in my head if its in one file. Its not a very large file right now. But I can understand it if it makes it easier for others to understand the codebase. I would be open to that change, but could you sketch out how you see the files being split, in a separate issue, before you open a PR? I ask since such a PR would get stale very quickly, so I don't want to keep it open for too long for discussion.

@aviks
Copy link
Collaborator

aviks commented Jul 7, 2015

Finally, Andrei, since you are using JavaCall heavily for Sparta.jl, I would be happy to talk over hangouts or skype if you wanted to clarify any design or implementation issue. My email is on my github account. Let me know if you find that useful.

@dfdx
Copy link
Collaborator Author

dfdx commented Jul 8, 2015

Finally, Andrei, since you are using JavaCall heavily for Sparta.jl, I would be happy to talk over hangouts or skype if you wanted to clarify any design or implementation issue.

Agree, it would be helpful, though currently I fill much more confident about the code and don't have a "global" questions right now :) Dicussing more local issues is better to do on GitHub so that others could also see and participate in it.

As you know, java.lang.Class{T} serves two purposes: one, as a description of the type of the object, and two as a collection of methods to operate on that type.

Just to complement the picture for myself and possibly others, there are 4 things in JVM that JavaCall supports:

  1. Type - primitive (jint, jdouble, etc.) or JavaObject{T}.
  2. "Dynamic" data - instance objects in Java, JavaObject{T}(...) in JavaCall.
  3. "Static" data - class objects in Java, JClass{T} in JavaCall.
  4. Methods - same meaning everywhere.

So, the reason I had an error is that I tried to use static data where type should have been needed.

I personally find it simpler to keep it all in my head if its in one file. Its not a very large file right now.

Well, for me keeping code in several (named) files just makes it easier to group related functions. E.g. when I first looked at the code, it wasn't obvious how init() (which is at the end of the file), uses findjvm() (at the beginning), and where JNI_VERSION_X is used at all. But grouping related stuff may actually be done inside the same file too! So how you fill about moving definitions into named sections inside the same file?

@aviks
Copy link
Collaborator

aviks commented Jul 12, 2015

So how you fill about moving definitions into named sections inside the same file

As I said above, I'd be open to a PR splitting this up as well, since now I am not the only person reading this code. However, if you can briefly talk about a proposed split before submitting a PR, that will help reduce merge conflicts from this.

@dfdx
Copy link
Collaborator Author

dfdx commented Jul 13, 2015

I imagine following files then:

  • jvm.jl -- house for JVM management functions: init(), destroy() and all related stuff
  • types.jl -- principal data types used in JavaCall: jprimitives, JavaObject, JavaMetaClass as well as JClass and JObject
  • jcall.jl -- all variants of jcall(), jnew() and supporting functions such as _jcall() or metaclass()

Though I'm not sure if it's possible to efficiently seperate types and jcall(), so it also makes sense to combine them into core.jl.

@aviks
Copy link
Collaborator

aviks commented Jul 14, 2015

All the convert* method should go together, possibly in a convert.jl. That'll make it easier to add additional convert methods, I think.

@dfdx dfdx mentioned this issue Jul 14, 2015
@dfdx
Copy link
Collaborator Author

dfdx commented Jul 14, 2015

Agree, especially given new conversions for date types.

So I will prepare PR for splitting files after #15 is merged.

I'm closing this issue since it's no longer concerns JClass.

@dfdx dfdx closed this as completed Jul 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants