Add support for sharing Java classes across the J2V8 bridge

[caer]

This pull request contains an assortment of utilities that I've written for use in the Alicorn platform (http://alicorn.io) that I wanted to share with you guys. Essentially, it adds basic support for easily sharing Java classes and interfaces across the J2V8 bridge including:

• Injecting Java classes and objects into a JS runtime and accessing them.
• Creating new instances of Java classes from the JS runtime.
• Invoking functional interfaces from the JS runtime and automatically creating proxy classes.

I don't know what your minimum performance requirements are for any APIs in J2V8, but I just wanted to share this work with you and hopefully get us closer to having full Java integration native to the J2V8 library. Tell me what you think!

Brandon

[irbull]

Wow, thanks! I may move a few things around (I want to keep the org.eclipsesource.v8 for core V8 abstraction layer and use other packages for the utilities. I'll go through this and get it in.

[caer]

My pleasure; I'm glad I could contribute some stuff back. Thanks so much for working on this awesome library!

[yofreke]

I gave a quick look through the code and wanted to clarify a couple things: Does this support extending a java class in js? Does this support declaring classes in js, and then using them in java?

We are currently using our own set of tools to accomplish this, and am interested in potentially using this implementation instead.

[caer]

I have not added explicit support for declaring/extending Java classes from the JS side.

That said, there is some capability for extension of Java classes in the context of functional interfaces. These utilities will automatically "cast" a Javascript function to a dynamic implementation (proxy) of a Java functional interface whenever you invoke a Java method that accepts a functional interface as an argument. While this certainly doesn't cover all of the possible cases for Java-JS bridging, it's been more than enough for the DSL work that we do with Alicorn.

However, when I have some more free time, I'd be happy to look into supporting actual extension of Java classes. It wouldn't be too hard to tack that kind of functionality onto the current set of APIs I've written, considering it would just be a logical extension of the current proxying behavior for functional interfaces. ;D

[BanderasPRO]

Hi, guys!
Thank you all for great job. It seems i've found some incorrect places in Mizumi code, please check for correction:

1. V8JavaAdapter.injectObject - the function doesn't bind a created object (V8Object other) with a rootObject field. I think before to invoke other.release(); the injected object must be placed into rootObjectby invoking rootObject.add(name,other);.
2. V8JavaObjectUtils.getRuntimeSarcastically has a typo 'getRutime' instead of 'getRuntime'

[caer]

Hey @BanderasPRO, thanks for reviewing the code!

1. I didn't catch that when I first wrote it...oops! The current implementation works, but I'm pretty sure that's only because I only ever bind to the top-level of the V8 run time. I think this would be trivial to fix if it's an issue, though.
2. Surprisingly, that's not actually a typo (thus the functions peculiar name). I don't know if it's still a problem, but on the Android version of J2V8 getRuntime was mistyped as getRutime. Therefore, I had to do a slightly reflective hack to invoke the correct method whenever the code was executed on that platform (in this case, getRutime).

[irbull]

Sorry, I started to look at this and got side tracked. I'm on vacation right now, which means a chance to look at my back log. I really like some of the threading ideas here. Let me put this back on the top of my priority list. Thanks again!

[caer]

No problem at all, I completely understand! It'd be great to get this rolling so that we can cut down on the number of people trying to solve these issues independently. ;D

[caer]

Hey @irbull ! Do you have an update on the progress of this branch and/or do you think its features will make it into the main code base sometime in the near future? It would be awesome to be able to use this as a part of the main release instead of tacking the features on as "hacks."

[irbull]

I've pushed this work to a branch on J2V8 to take a look and get it included in master. See https://github.com/eclipsesource/J2V8/tree/mizumi-java-adapter

[irbull]

Sorry, pressed enter too quickly. I've also rebased this against master so it's up to date.

[caer]

Awesome, just let me know if you need anything from me. Thank you so much for working on this!

[irbull]

I am splitting this up a bit to make it easier to review. I made a few changes to the ConcurrentV8 and pushed it to a new branch. https://github.com/eclipsesource/J2V8/commits/concurrent_v8

@mizumi Can you take a look and give me your thoughts. I modified a few things, namely:

• I have removed the run method that suppressed the exceptions. I don't think it's every a good idea to catch (and swallow) throwable.
• I have removed all the catch blocks and instead cleaned up the locker in a finally block (this means we don't need to catch and re-throw or wrap exceptions).
• I have removed the finalizer. A finalizer is not guaranteed to be called, and for that reason, we don't use them in J2V8. I think we should be consistent with the rest of the code base.
• I have changed it to use the existing V8Runnable (instead of creating another runnable).
• I have moved the ConcurrentV8 to the utils package.

[irbull]

The specific commit is 3a4e7e0.

[irbull]

A few "High-level comments" about the rest. I will try and address some of these tomorrow:

1. There are lots of Static object containers (static fields that hold objects or V8Objects). These are in places like the Cache and even in the utils. This can lead to problems if J2V8 is used in places like Tomcat. Furthermore, if we hold V8Objects in these, we can easily run out of handles. All holders of V8Object must be managed.
2. The cache needs to bind itself to a V8Runtime. We cannot share V8Objects between runtimes (A v8 limitation), so we should ensure that we tie the cache to the lifecycle of the runtime.
3. We should never store a V8Object (that anyone else has accessed) in a collection. If someone ever calls release on a reference to that object, we will be in trouble. If we must hold a V8Object, we should hold a twin() instead. But remember, there is a limit to the number of objects stored, so the collection should never grow unbounded.
4. I don't really understand getRuntimeSarcastically. Why would getRuntime() throw an exception, and if it did (outOfMemory error, or runtime released) calling the old method won't help.

I am moving most of this code to a package called com.eclipsesource.v8.bridge.

If you have any thoughts on those comments, let me know. I will work on this again tomorrow. Thanks @mizumi!

[caer]

@irbull Your changes to the shared V8 look great; I definitely don't have any issues with them.

Addressing your high-level comments:

1. I'm not 100% sure why I made everything static in the first place... I think it may have been for convenience, but I'd have to look back to the code to be sure.

2. That makes complete sense; once everything is non-static, I imagine that's trivial.

3. I'd have to look back to the depths of the code, but as I recall, I store the V8Objects expressly to enable binding back to their original Java objects (or vice-versa). I'm not sure how to get around this, but I could look if needed.

4. The getRuntimeSarcastically method was written due to a typo in the Android SDK. It's not meant to eat exceptions that would occur naturally (although it does that, too, to avoid screeching halts in favor of just passing back null pointers). If that typo has been fixed, this method can go away.

Thanks so much for working on this, and let me know if you have any further comments!

[tusharwd]

@irbull when we can expect a artifact to be available with this change. Or is it already published ?

[caer]

@irbull Following up from @tusharwd , is there any reason this artifact can't be merged yet? It would help avoid any more duplicate efforts...please let me know if there's something I can do to help speed this up.

[jonnermut]

Needed for lots of use cases, can this be merged and released, please? Is it usable in its current state on the branch with a manual build?

[caer]

@irbull puppy eyes

[caer]

@jonnermut Yes, it can be used on the branch with a manual build so long as you update it with master. The conflicts are minor (just a gitignore).

[aschrijver]

This seems like a really good PR. Are there any things blocking its adoption?

[drywolf]

I will put my 2 cents in here...

While I would very much welcome J2V8 getting improved for use in multithreaded environments (I'm talking about the efforts to improve concurrency support in this PR) ... I am also a big advocate of separation of concerns (talking about the class-sharing parts)
That being said, when asking the question of what should be in and what should be out of J2V8 I would say that J2V8 should stay focused at providing a rather low-level interface between the JVM and Node.js/V8

To give an example ... I am thinking of this as a similar case as of how modern programming languages nowadays are separated from their base-class libraries / framework-classes. A clean separation of what is a low-level systems interface and the emerging tools & utilities that are built on top of that, yield so much better results in maintainability, code quality, user acceptance, lower barrier of entry for contributors, etc.

PS: as far as I am aware there are already at least three people/projects that are (/were) looking into implementing some kind of class interop on top of J2V8. And all of the implementations are kind of similar, but also very different in some aspects ... this should be a strong indicator that we would be better off implementing such features outside of J2V8 and would also decouple potential collaborations between those projects from the J2V8 PR/Release cycles (meaning that development of such features could be way more iterative and fast-paced rather than having to wait for huge PRs to be merged into J2V8 and which would need to reach acceptance of all interested/affected parties)

TLDR: I would say that J2V8 should stay focused on delivering only the low-level primitives to communicate between the JVM and Node.js/V8 and everything else that provides higher-level API / programming features should be kept as separate projects.

[caer]

@aschrijver Only time and, more critically, any decisions regarding the direction that J2V8 itself wants to take. It's likely that this PR will become it's own project; it's proven itself quite well over the past year on Alicorn and is more than ready for use in production environments...in my slightly biased opinion. ;)

@drywolf I agree that a separation of concerns - both within this pull request and without - is important. We really don't want to bloat J2V8 with unnecessary stuff.

However, almost all of the major Java-based JS engines support a form of explicit sharing of Java classes between runtimes; this is why I feel it's appropriate to include these classes within J2V8. If it is not, and @irbull and you agree that it should stay low level, I would be more than happy to turn this into it's own Maven project that builds on J2V8 without bloating the original. This PR has been around for a year now, and I'm at a point where I'll do anything to get it into the hands of the community...

Let me know @irbull @drywolf, and I'll get the ball rolling!

[drywolf]

However, almost all of the major Java-based JS engines support a form of explicit sharing of Java classes between runtimes; this is why I feel it's appropriate to include these classes within J2V8.

I can relate to this very well, actually this would be my primary usecase for J2V8 anyway, that I would continue to work on once I run out of ideas for features to add to the build-system #327

Therfore I would not count out the possibility of having this feature in the J2V8 core (but actually this is totally @irbull 's call ATM) ... nonetheless what I would add to my argumentation from above is that even if we decided to include this feature in the J2V8 core code, then it would have to be as little opinionated as possible in how it can or must be used.

By this I mean that every bit of "magic" or "non-standard" syntax that is needed to work with this feature (integrating Java Objects with JavaScript objects and the other way around) should be reduced to a minimum or better completely avoided, unless there is a really good reason that something can not be done without it. This is to make sure that this feature can be easily used in as many potential usecases, that J2V8 users might face in their applications of J2V8, as possible.

Also there should be a discussion of what are the features that this class-interop should support, as well as a clear documentation/specification/unit-tests definition to make sure that, if we really put it in the J2V8 package, we really get it right for everyone to use.

But until we go any futher into these theoretical discussions I would wait until we hear from @irbull what his take is on this general topic. 😄

PS @mizumi Do not get me wrong, I really like what you have done there 👍 ... especially the functional JS parts, which I would not have thought of yet ... I might come back to you about those parts when I finally continue work on my own hobbyist J2V8 interop project 😉

[irbull]

Hi everyone, sorry for the radio silence on this issue. I've had to focus on a few other things and this one keeps slipping down. However, I really do have this in my workspace (and it's been there for months) and I've taken a few stabs at some of the parts that worry me. In particular, all the static calls.

The concurrency part has been merged already, so this is really about the object sharing.

I do understand that many people wish to share Java objects easily, and having J2V8 support would be a big step forward in terms of ease of use. However, I have always been worried about leaking memory handles, which is why J2V8 requires explicit object management. If we are endorsing this as a way to work, we need to be sure that we are handing memory properly (otherwise we'll fielding issues related to out of memory errors and JVM crashes).

Having said all that, I just recently added V8Value#setWeak. I currently don't have a callback that we can use when a V8 object is cleaned, but that should be easy to add. With that a java.lang.ref.Cleaner we should be able to handle the memory clean-up, assuming we get all the notifications.

The other concern I have is project size. This is an optional way to work with J2V8, so I don't think we need to include it in all builds. Since there is no native code, pulling this into it's own build isn't very difficult. Maybe we should start to consider how we can structure J2V8 in terms of a core, and a set of libraries. Arguably even the current V8ObjectUtils can be moved out, leaving just the JNI Bridge in the core. Does anyone have thoughts on that?

[aschrijver]

I have just one cent to contribute, as I'm new to J2V8, but here goes..

@drywolf - I am also a big advocate of separation of concerns (talking about the class-sharing parts)

[...] I would say that J2V8 should stay focused on delivering only the low-level primitives to communicate between the JVM and Node.js/V8 and everything else that provides higher-level API / programming features should be kept as separate projects.

@mizumi - I agree that a separation of concerns - both within this pull request and without - is important. We really don't want to bloat J2V8 with unnecessary stuff.

@irbull Maybe we should start to consider how we can structure J2V8 in terms of a core, and a set of libraries.

Yesss!! I am 100% for this direction.

To be frank I was worried on the state and direction of J2V8, and the PR's hanging around for so long (mentioned in the 'cons' section of my evaluation). So having as many potential useful features merged was a good thing. But J2V8 would be so much more helped with a razor-sharp focus and purpose!

In terms of good project structure and community approach I have an excellent case study for J2V8 and that is Vert.x. Please read this issue I created for a similar purpose for the Dat Project:

• Vert.x: A case study of successful project reorganization

I intend to use Dat technology and are altogether more worried about their development approach than I am with J2V8. They practice what you could call 'programmer anarchy' and are not yet that interested in restructuring.

The Vert.x way would fit very well though on a Java project like J2V8, much more than in node world with all the micro-module development going on.

One important thing to add is that - looking at the vertx.io landing page - if you had such good design it would be an excellent location to more prominently attract attention to commercial products and professional services on top of J2V8!

---

PS I am here with non-commercial motives and the satisfaction of spending the last part of my sabbatical on open source work. The (non-profit) solution I am investigating seems best served by J2V8. My philosophy is that open-source users should show gratitude back to corporate contributors by providing feedback that helps their business.

[matiwinnetou]

Functionality provided in this PR does not seem to be necessary for core operation of neither V8 nor NodeJS. I would vote to keep this as a submodule at most, if my vote plays any role. The exception could be concurrentV8 but that is already merged to master.

[aschrijver]

I have created a separate discussion issue for J2V8 project-related stuff, so this can be on the PR again.

[caer]

This feature has now been extracted from the V8 repository and included in a stand-alone project here: https://github.com/alicorn-systems/v8-adapter. Note that the new project has its own Maven coordinates and is under a different license, and is fully compatible with the latest J2V8 version.

This should address the concerns mentioned here and in #335 regarding scope of the V8 project etc. Please let me know if you have any questions; I will leave this PR open until I can hear from @irbull about whether or not this is an appropriate direction for these features!

[aschrijver]

@mizumi good to have this split into a separate repo.

Personally I would prefer a common organization (either eclipsesource or j2v8 or whatever) where the OSS v8 repo's reside in one location (e.g. like this: https://github.com/vert-x3 ).

This would be much more comprehensible and likely to attract more contributors, better from a community building perspective.

In the current setup, you would need an awesome page to track what's going on. Also possible, but less attractive, imho :)

[qiaoyupeng]

@crahda hi，can i ask you for some advice? im wondering if i inject a Image Class to js though v8Adapter, is there any chance i could konw about the gc time of corresponding js image object so i can free the jave image object, i use V8value.setWeak() or set weakcallack on the Persistent, but still the callback wont get excuted, i doubt it maybe the java v8value holds the objecthandle so the c++ object cannot be released?