Not picking up the previously-used proxy when getting Ruby Classes from Java

The same problem when we define an java class implementing an java interface like...

public class SimpleJavaClass implements JavaInterface {
public String get()

}

and we receive an instance of the class in jruby, like...

object = JRuby::getJavaClass()

puts object.kind_of?(SimpleJavaClass) => true
puts object.kind_of?(JavaInterface) => false

where getJavaClass is an java method that returns a new instance of SimpleJavaClass.

I just noticed that this bug also prevents ruby methods from being called once the object returns to ruby.

So something like this breaks :

class Xpto < JavaInterface
def description
"AAAA"
end
end

x = Xpto.new
puts x.description # this works fine
x = JRuby.getJavaClass # this is a method in java that only returns it's argument
x.description # this doesn't work

NoMethodError: undefined method `description' for #<#<Class:01x17e319a>:0x18ba593 @java_object=$Proxy1>
from (irb):1:in `method_missing'
from (irb):1:in `binding'

This one is super tricky to support efficiently right now. Given that the wrapper can't be passed out to Java, we'd have to keep a map of every single object we've seen to the wrapper we used to access it. We already do this for class proxies, so we can maintain the appropriate proxy class to wrap a given Java object with, but it would double or triple the object count per Java object entering JRuby if we had to also track the wrapper objects we use in JRuby. It would also mean that every single Java object passing into Ruby would incur another hash hit for the wrapper cache, in addition to the hit for an appropriate proxy class. That seems like it would be unacceptable load to add.

I think the best solution for this may unfortunately have to come after 1.0, when we can move to a more Groovy-like model that allows Java objects to be passed around directly, with only their metaclass information being Ruby-specific. At the moment I don't know of a feasible or efficient way to track every object's wrapper across calls where we can't easily maintain that association.

Actually... we're already caching the JavaObject wrappers (along with the wrapped objects) for all Java objects being passed in to Ruby (in JavaObject#wrap). I don't really see the value in this (since plain [non-class] wrappers shouldn't carry any state), though I recall there was a thread long ago in which someone complained wrappers weren't being matched correctly, so perhaps it matters to someone. I could maybe see caching a few wrappers for reuse without matching them to particular objects. (BTW, after loading Cheri::JRuby::Explorer, there are about 700 object/wrapper pairs cached; after using it for a minute, that goes up to over 7000...)

Truly absurdly, we also wrap (and cache!) outbound primitive values (parameters to Java methods/ctors).

Anyhow, it seems to me we could (instead?) cache the proxies for non-primitive, non-String, outbound Java objects (in Java.ruby_to_java), and check the cache for non-primitive, non-String, inbound Java objects (in Java.new_instance_for). We'd only be caching those objects passed out to Java, which (I think) would normally be a minority.

I agree that better solutions lie ahead in the revamped JI model, but in the meantime, this could be fixed pretty simply. Thoughts?

We need to decide on a course of action for 1.1

Has anything been decided yet? I'm interested in the eventual implementation due to problems we've encountered with deadlock from the old JavaObject wrapper locking scheme: JRUBY-1559.

I've got the fix mostly done, haven't had time to wrap it up. Will try to get if finished this week.

-Bill

I've committed a fix for this on trunk (r5360). Proxies are cached in a Weak/Concurrent Map-like structure, which should provide reasonably good performance (not verified yet). The old JavaObject instanceCache has been removed, as it serves no real purpose; since that was synchronized (whereas the new cache uses concurrent techniques), I would expect a net performance gain, though again, I haven't verified this.

One issue I've noted in the code is that the new cache holds weak (or soft) references to both the underlying Java object and its proxy. This is necessary because the proxy has a strong reference to the Java object, so nothing would ever be gc'ed if we kept a strong reference to the proxy. That means that at some point an incorrect proxy could be returned from Java, unless the user keeps a reference on the Ruby side to any proxies passed to Java. There's really not much that can be done about that, at least under the current architecture.

I'm leaving this issue (and the related/duplicate JRUBY-1513 and JRUBY-1735 open pending the addition of some unit tests, and verification that this actually solves the problems reported. It would also be good to do some profiling, especially WRT memory consumption. I've currently got the cache using soft references, but this is a parameter to the cache and can be changed easily to use weak references if the memory hit is too great.

[As an interesting side note, it appears that at some point JavaObject was meant to be the base class for all "proxy" classes; the old instanceCache did for JavaObjects exactly what I've done for proxies, if somewhat less performantly. However, under the current architecture, it appears to serve no good purpose.]

[A second side note: I spent some time the past few days looking at eliminating out extra layer of wrapping, and in fact making JavaObject the base class for all proxy classes (but not interface modules), and also hard-wiring it as the Rubified version of java.lang.Object. I like the idea as a transitional step in our JI overhaul, simplifying and eliminating a lot of cruft. But it may be a bit much in the midst of a beta cycle. Also, after next week (when I get a few more extra days), I'm not sure when I'll next have much time.]

• Bill

I think most of us agree that the eventual path for Java integration is to elimininate as much as possible of the plumbing and move toward a lightweight approach for object in general, so there's no difference between Java and Ruby objects. That's likely to be the big focus after 1.1, leading up to a big and potentially final major game-changing release of JRuby for JavaOne. But for 1.1, ideally the focus would be in just simplifying the JI layer, eliminating genericity that's not useful now that most of JI is implemented in Java, and improving performance and consistency across all typical JI use cases. That much we can do for 1.1, and that's where we should put the focus. Assume the current code is staying put and make it as lean, clean, and simple as possible.

In rev. 5606, backed off using soft references in favor of weak references in ObjectProxyCache, due to excessive heap use incurred by some applications, as reported in JRUBY-1955.

This remains a temporary and less-than-satisfactory solution, now even less satisfactory – even with soft refs, there was no guarantee an object returning from Java would be mated with its original proxy, unless the user maintained a reference to the proxy in Ruby, as it might otherwise have been GC'ed in the interim. But the odds were much better than with weak references. Now, the odds are slim, and users will essentially have to do their own caching if they need to guarantee returning objects are correctly mated.

It should be noted that this issue pertains only to Java objects that have some form of Ruby-only state: any instance of a Ruby-defined subclass, and those that define instance variables, singletons/methods, constants, and so on. In theory, this could be detected and only those objects cached; in practice, checking all outbound (non-primitive) objects would be expensive (I think; I may try this, time permitting).

The real solution will come with lightweights, when proxies and JavaObject wrappers go away; any references to Ruby-only state can be kept in a global map keyed by a weak reference to the Java object itself. (See the Registry class in my experimental bnw branch for a [partial, out-of-date, and not terribly efficient] proof-of-concept implementation: http://svn.codehaus.org/jruby/branches/bdortch/bnw/src/org/jruby/bnw/Registry.java)

-Bill

Punting issues from 1.1 RC2 to 1.1 final.

Bill's mostly fix is already in for this; and the weakref/softref debate will probably not be solved until we can avoid using wrapper objects entirely. So I'm punting this to post 1.1 for that work, since we won't be doing any more on this in 1.1.

This has been fixed for some time; the only thing keeping it open was further work and discussion on the mechanism for its resolution. Given Bill's comments (http://jira.codehaus.org/browse/JRUBY-199?focusedCommentId=119902#action_119902) I will file a separate bug and mark this as resolved in 1.1.