Nice! I'll review tonight.

🌲 👍

Finally getting back to this now.

Ok, this isn't quite right. This will unconditionally cache objects coming from all constructor calls whether the proxy cache is on or the class has been set to persistent.

If you look at InstanceMethodInvoker and trace through method.invokeDirect, after a long chain of calls (@kares we need to reduce this) it eventually gets back to Java.getInstance() which checks those flags and decides to cache or not.

This might be ok if we modify setAndCacheProxyObject to check those same flags.

@headius thanks for taking a look! Sorry I didn't look deeply enough into the InstanceMethodInvoker path!

I'll take another pass and see if I can fix these problems, along with some more tests.

@headius I'm wondering about something like this:

    private void setAndCacheProxyObject(ThreadContext context, JavaProxy proxy, Object object) {
        proxy.setObject(object);

        if (Java.OBJECT_PROXY_CACHE || Java.getProxyClassForObject(context.runtime, object).getCacheProxy()) {
            context.runtime.getJavaSupport().getObjectProxyCache().put(object, proxy);
        }
    }

However, I notice there is a RubyModule clazz parameter to all the call methods... is that already the proxy class I seek? I'll dig in a bit more, but if you know off-hand, that would be a big help :-)

right, did not realize. will fix it unless @smellsblue spins up another PR.

@smellsblue I believe that clazz is the class on which we're calling new, so you should be able to check for the class proxy directly.

@kares @headius I just created a new PR to both fix the bug and add some specs around proxy caching: #3602

@headius using the clazz parameter seems to work, so I went with it... if you think there might be a scenario where the clazz isn't the right RubyClass, please let me know, though I may need some guidance on how to check if it is the right RubyClass before fetching it.