search

JuliaInterop / JavaCall.jl

Call Java from Julia
http://juliainterop.github.io/JavaCall.jl
Other
118 stars 53 forks source link

JProxy update #112

Closed mkitti closed 3 years ago

mkitti commented 4 years ago

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

zot commented 4 years ago

Thanks a ton for doing this!

zot commented 4 years ago

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

mkitti commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

  1. I switched most of the JNI calls over to the JavaCall.JNI module which wraps the ccalls
  2. 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.
  3. 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.

aviks commented 4 years ago

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

mkitti commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

Is this dying on the vine?

mkitti commented 3 years ago

Most of the changes to core JavaCall were merged in b5692cc14d7830283b90383a3b15fe752518c47e 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 commented 3 years ago

Add a JProxy subpackage