sagax has quit [Read error: Connection reset by peer]
sagax has joined #jruby
ur5us has joined #jruby
ur5us has quit [Ping timeout: 240 seconds]
xardion has quit [Remote host closed the connection]
xardion has joined #jruby
subbu is now known as subbu|lunch
<headius[m]>
kares: I know you don't have a lot of off hours to work on JRuby lately but we need to brainstorm a proper way to bind the methods we need
<headius[m]>
enebo: maybe you are around today to chat a bit
<enebo[m]>
headius: sure
<headius[m]>
I'm wondering if maybe we should return to using getMethods() which will return all publicly callable methods on a given class... it would be duplication from parent to child but we'd know we have all callable methods in hand for a given class
<headius[m]>
I worry about efficiency (binding all publicly callable methods on every class will have tons of dupes), startup/boot time, and we would need a separate pass if we want to also bind non-public methods for whatever reasons
<enebo[m]>
hmm
<headius[m]>
there's a possible perf gain if we save time by letting JDK figure out those public methods for us
<enebo[m]>
so if we defer to a subclass proxies version it does not know its original type so we mess up visibility
<enebo[m]>
well perf gain seems pretty unimportant since we would site cache up to caller right? It would only get rid of first search
<headius[m]>
right, the example I have in the PR shows that the internal "Itr" type in ArrayList won't allow us to call `next` etc
<headius[m]>
right, it's largely boot time method binding perf
<headius[m]>
just to get all the cards on the table, the case we don't support right now is calling a public method inherited from a non-public superclass when the child is public
<headius[m]>
that's the original bug from fidothe
<headius[m]>
the case that breaks with binding everything is the ArrayList::Itr type stuff, where you have a non-public impl of a public interface
<enebo[m]>
refresh my memory here....the original move away from binding all methods per leaf class was just an optimization or did we do it for more than that?
<headius[m]>
it was because of the non-public child case
<headius[m]>
if we bind the child's version of the methods, dispatch blows up because it thinks we're trying to call a method on a non-public class
<headius[m]>
oh but you mean all methods as in getMethods output?
<headius[m]>
that was optimization
<enebo[m]>
yeah
<enebo[m]>
ok
<headius[m]>
fewer methods bound, and ideally trying to only bind names on classes that define them
<headius[m]>
so here's interesting output from getMethods for the fidothe case:
<enebo[m]>
Is it possible to merge the two methods? At the time we make a leaf we know all parents and the basic visibility. So we could getMethods and look down and then only bind the problem methods at the leaf?
<headius[m]>
public java.lang.String java.lang.Object.toString(), public native int java.lang.Object.hashCode(), public final native java.lang.Class java.lang.Object.getClass(), public final native void java.lang.Object.notify(), public final native void java.lang.Object.notifyAll()]
<headius[m]>
[public void InheritsFromAbstractClass.someMethod(), public boolean InheritsFromAbstractClass.aMethod(), public final native void java.lang.Object.wait(long) throws java.lang.InterruptedException, public final void java.lang.Object.wait(long,int) throws java.lang.InterruptedException, public final void java.lang.Object.wait() throws java.lang.InterruptedException, public boolean java.lang.Object.equals(java.lang.Object),
<headius[m]>
note that "aMethod" there is actually defined on the superclass, but reflection gives us a callable version on the child
<headius[m]>
yeah I guess I don't know what all the "problem methods" are
<headius[m]>
this case escaped our attention forever
<headius[m]>
I think it was only "fixed" in 9.2.9 because I "broke" the logic that filters out non-public classes
<enebo[m]>
heh..I can say anything public all the way down is not
<enebo[m]>
so I think at a minimum we could not bind those at leaf types
<enebo[m]>
perhaps we can learn enough to bind even less over time but that would eliminate a lot of extra binding no doubt
<enebo[m]>
My heh was the reaction I don't know either :)
<headius[m]>
I just noticed something about that output: it only rebinds the methods that couldn't be called again the original class
<headius[m]>
Object.wait for example still says it's Object.wait
<headius[m]>
So maybe we can use getMethods output to find all classes it actually considers bound locally to that class
<enebo[m]>
main it is doing the hybrid naturally?
<enebo[m]>
main heheh
<enebo[m]>
maybe
<headius[m]>
that still leaves the open question of any methods we want to make it accessible, but it might cover the publicly callable cases completely
<headius[m]>
Perhaps I should also look at the implementation of getMethods and see if the heuristic is simple enough to copy
<headius[m]>
It may be yes
<headius[m]>
They are essentially doing the same class walk that we are, but presumably they're closer to correct
<enebo[m]>
I guess at some level this binding knowledge is all in Java already so replicating the same logic probably will not make it slower and if it can be combined with other heuristics it will be faster
<headius[m]>
So I'm thinking we get methods for each class but then only bind of the ones that should be locally bound, according to where they claim to be located
<enebo[m]>
I am not terribly worried about extra methods on types but in Purugin that framework loads hundreds to thousands of types
<enebo[m]>
So it probably is nearly a worst-case project
<enebo[m]>
I never noticed anything change but I was not measuring anything either
<headius[m]>
yeah there's some class hierarchies out there that will have thousands of methods
<enebo[m]>
jrubyfx actually loads a lot of Java classes as well
<enebo[m]>
but we I believe already do crap loads of replacement bindings
<enebo[m]>
since there is a lot of magic in handling incoming params
<enebo[m]>
if only we had Groovy's MOP
<enebo[m]>
(is only joking)
<headius[m]>
hah
<headius[m]>
well I guess I'll poke at getMethods for a bit and see how it looks
<headius[m]>
gods I must never have looked at getMethods... the algorithm is almost what we have
<headius[m]>
I am actually a bit surprised now... I just made this change and I don't see aMethod bound on the child class, but it's bound on parent class and still callable
<headius[m]>
[public void java.util.ArrayList$Itr.remove(), public void java.util.ArrayList$Itr.forEachRemaining(java.util.function.Consumer) ...
<headius[m]>
so this still gives me methods that aren't callable
<headius[m]>
I've struggled with this algorithm since the beginning
<headius[m]>
I'm back to thinking my original patch is the right one
<headius[m]>
public methods on a non-public superclass are basically the same as public methds on an interface... you can't dispatch directly against the superclass or the interface, but you can dispatch against the child and it should get bound there
<headius[m]>
the algorithm we want is basically this..
<headius[m]>
1. bind all methods callable on a given class
<headius[m]>
2. don't bind methods that are callable (and bound) in a superclass
<headius[m]>
2 is maybe optional and only an optimization
subbu|lunch is now known as subbu
<headius[m]>
you know I think we may just have to read up on the Java language spec for how it decides what method signature to call with, and duplicate that logic when binding the Ruby proxy class
<headius[m]>
I was about to tweet asking if there's any spec for how to know all callable methods on a given target class, but then I realized that's basically what javac has to resolve