_whitelogger has joined #jruby
ruurd has quit [Quit: bye folks]
den_d has quit [Ping timeout: 264 seconds]
Liothen has quit [Read error: Connection reset by peer]
Liothen has joined #jruby
Iambchop has quit [Ping timeout: 272 seconds]
den_d has joined #jruby
Iambchop has joined #jruby
<headius[m]> good morning!
<headius[m]> enebo: around?
<headius[m]> I have had two moments in the last week that I wished we had concrete Java exceptions to represent errnos so I might spike a PR to try that
<headius[m]> currently they bottom out in SystemCallError so you can't catch individual errno types
<headius[m]> hmm maybe not all of them
<byteit101[m]> I got excited when I saw the word concrete, then realized it was something else :-)
<headius[m]> haha
<headius[m]> byteit101: ok it is time
<headius[m]> heh # TODO: doc
<headius[m]> I was going to ask if configure_java_class had doco but figured I should check the diff
<headius[m]> enebo: would like your opinion on the concrete class configuration DSL here: https://github.com/jruby/jruby/pull/6422#issuecomment-743924535
<headius[m]> it seems fine to me but maybe some namesmithing is in order
subbu is now known as subbu|away
<headius[m]> > How should I set allocators, the new method, and initialize? new seems to be magical on ConcreteJavaProxy instances, and the way I do it seems very heavy.
<headius[m]> can you point out where this is in your PR?
<byteit101[m]> Yea, I avoided writing doc until names are finalized
<headius[m]> got it
<byteit101[m]> Ah, several places, one sec...
<headius[m]> is this just a concern about setting up those methods when generating the class? That is already so heavy I would probably not worry to much
<headius[m]> if it is more about the methods themselves being heavy that is a different matter
<byteit101[m]> RubyClass 1382-1406 (calls to set*Allocator), JavaProxyClass.setProxyClassreified 546-561, ConcreteJavaProxy.StaticJCreateMethod.tryInstall, and ConcreteJavaProxy.initialize. Magical is referring to the last one, which is called once very early, vs per each class for the other areas.
<byteit101[m]> My code (first 3) is just overwriting those methods, which I wasn't sure if it was a good way to do it, as it might hide other custom new/initialize, hence the check in JavaproxyClass with instanceof
<headius[m]> ok I will have a look
<headius[m]> byteit101: "What methods & ctors should we define? See above comment as well"
<headius[m]> what do you mean... are you asking about other methods that should get defined as part of Ruby protocols like initialize_copy?
<byteit101[m]> Oh, not that, just which methods and ctor signatures to mirror on the java object to proxy back to the ruby object when used in java-land
<byteit101[m]> (the first half, the signature matching)
<byteit101[m]> (oh, and "Generate all or only necessary?")
<headius[m]> ahh right that one
<byteit101[m]> comment updated with link for future reference
<headius[m]> so by default the old logic basically does every method, yes?
<headius[m]> now I am confusing myself... I may be thinking of interface impl which has to impl everything
<byteit101[m]> No, it does some weird most-but-not-all
<byteit101[m]> hence why those tests in Gaoon? fail on both my current min and max settings
<byteit101[m]> gaoon? = Generate all or only necessary?
<headius[m]> I am leaning toward only necessary
<headius[m]> so here is my thinking... in order for us to override all methods so they can be redefined in the Ruby class later, they would all need to touch some Ruby plumbing every call. It seems wrong to do that when user has no interest in overriding a given Java methods with Ruby code
<headius[m]> user should opt into overrides before they start using the concrete subclass
<headius[m]> so then I guess the question is about existing behavior and whether we break it by only proxying the opt-in overrides
<headius[m]> I was under the impression that your configure_java_class defaults of `all` were doing this though, is that not the case?
subbu|away is now known as subbu
<byteit101[m]> The question in that case is: which is the default? `:all` does it, but it's more than previously (tests fail with too many methods defined). The other option is fewer than currently (tests fail with too few methods defined)
<byteit101[m]> (i need to run that test again, I forget what the exact eerror is)
<headius[m]> right ok
<headius[m]> byteit101: in the generate all case you say "internal class methods are exported as well"
<headius[m]> what methods do you mean
<byteit101[m]> public static IRubyObject const_missing(IRubyObject... var0) {
<headius[m]> oh I see
<byteit101[m]> etc... (only if defined)
<headius[m]> I am still confused... that is a static method
<byteit101[m]> Ooops, one sec, running the test
<byteit101[m]> hmm... I don't think I solved that problem, but I'm not seeing it right now. But I do remember seeing "initialize" separately, as well as __jcreate!
<headius[m]> I would only expect methods from the superclass to get overridden
<headius[m]> as far as the Java side goes I don't think we need to export any of the internal JRuby signatures
<byteit101[m]> Is that list complete and correct?
<headius[m]> ah good... that seems like a pretty good list
<headius[m]> I would say anything on BasicObject also should be preserved too, like send and object_id perhaps
<headius[m]> `__send__`
<byteit101[m]> for export to java?
<headius[m]> filtered I mean
<byteit101[m]> (note concrete extension doesn't derive from basic object)
<headius[m]> ah yeah I suppose that is not a problem
<headius[m]> so it seems like you are filtering fine then
<byteit101[m]> If you have more things, add a comment with details and I'll add them
<headius[m]> ok
<headius[m]> oops hit send too early but there are some answers on the issue now
<headius[m]> continuing...
<byteit101[m]> Ah, nice
<byteit101[m]> Oh, btw, not sure if I mentioned it or not, but IRO constructor == (ConcreteJavaProxy rubyobject, boolean inherited, IRubyObject[] args, Block var4, Ruby var5, RubyClass var6), not (Ruby var3, RubyClass var4)
<headius[m]> ahhh ok
<byteit101[m]> (IRO because it takes an IRubyObject, it doesn't allocate a new one)
<byteit101[m]> Oh, want a decompile dump?
<headius[m]> > Where to stick the static int lookup?
<headius[m]> what is this referring to
<byteit101[m]> You want a decompile dump :-)
<byteit101[m]> one moment
<headius[m]> ok
<byteit101[m]> static int: https://gist.github.com/byteit101/ef20fefd119984524884c4629e67df79#file-decompile-java-L105 Generator: RubyClass 1894/reifyClinit
<byteit101[m]> 1899 generates the int
<byteit101[m]> gist is the ruby source from my testing hacky script, and the resulting FernFlower decompilation of the class file
<byteit101[m]> right now the static int lookup is in JavaProxyClass, but it felt weird
<byteit101[m]> '11' in the dump is a runtime number, classes are only valid in that runtime
<headius[m]> oh static init
<byteit101[m]> no, static int in the static init
<byteit101[m]> :-)
<headius[m]> heh yeah
<headius[m]> reading the code for this now
<headius[m]> ok right, is this equivalent to the old code's use of threadlocals to pass the runtime and class around?
<byteit101[m]> No. Back in october, you were wanting to get rid of the static `clinit(Ruby, Runtime)` function so as to make ruby/rubyclass final. This is my solution to that
<headius[m]> ahh ok
<headius[m]> so rather than them being mutable and calling clinit to set them this puts them in a known place and the real <clinit> grabs them and assigns them to finals
<headius[m]> maybe a threadlocal would be simpler? We proceed to initialize the class immediately
<byteit101[m]> Classloaders aren't multithreaded?
<headius[m]> they are but there's only going to be one concrete subclass getting initialized on a given thread at a time
<headius[m]> so we stick it in a threadlocal, force the class to init, and clear the threadlocal so nothing leaks
<byteit101[m]> cool, can change that if you think it's a better design
<headius[m]> yeah one less collection to manage
<byteit101[m]> :-)
<byteit101[m]> I like that solution
<headius[m]> and then no worries about the static int and such
<headius[m]> I will also comment and continue
ur5us has joined #jruby
<byteit101[m]> Thanks!
<headius[m]> > See clone question above: #6422 (comment)
<headius[m]> I have never considered what happens if you clone a Java class proxy so I think whatever makes sense here is fine
<headius[m]> I would almost argue they should not be cloneable since we are not actually creating a duplicate Java class
<byteit101[m]> I hadn't though of it until I saw it in the ji tests
<headius[m]> there are some weird cases in there... I am not sure why this is even needed
<byteit101[m]> spec/java_integration/fields/field_accessor_spec.rb:18
<headius[m]> ah thanks
<byteit101[m]> I didn't want to touch the test, so attempted to fix the clone
<headius[m]> ahh so this is cloning to avoid exposing those fields in other tests
<byteit101[m]> cloning does reset the name, so you do get two class fiels
<byteit101[m]> *files
<headius[m]> 😳
<headius[m]> where do we do that
<headius[m]> ah JavaProxy
<headius[m]> wait no that is just initialize_copy on the object itself
<byteit101[m]> It resets the reified instance, so we reify again using the new name
<headius[m]> so it becomes a new subclass of the old class's superclass and that is why methods are left abstract
<headius[m]> yeah this is unexpected behavior
<byteit101[m]> yes, I think
<headius[m]> I think we should make a decision about how this should actually work because it is not behavior I have ever specified myself
<byteit101[m]> took me awhile to make sense of itmyself with irb
<headius[m]> hmm I am leaning toward it being a soft clone that just gives you a new named proxy wrapping the same class
<headius[m]> so ArrayList.clone would produce a new proxy but it still just represents ArrayList
<byteit101[m]> > so it becomes a new subclass of the old class's java superclass, and a clone of the ruby class itself
<headius[m]> the only use case we have in tests for this seems to be trying to isolate changes
<byteit101[m]> ^ not a quote, but an edit
<byteit101[m]> which is why it isolates, but the parent behavior is clearly wrong
<headius[m]> right
<headius[m]> I think cloning a reified class should keep the reified class
<headius[m]> that just seems to be an oversight in the RubyModule.initialize_copy code
<byteit101[m]> (commit later cleaned up, just check latest rubyclass on that PR)
<headius[m]> yes that seems right
<byteit101[m]> if (originalClazz.getJavaProxy() && originalClazz.reifiedClass != null && !Reified.class.isAssignableFrom(originalClazz.reifiedClass)) // TODO: ensure this line is correct
<byteit101[m]> ^ that only copies non-ruby reified parents (ie no concrete extension)
<byteit101[m]> Is that good?
<byteit101[m]> or do we also want to clone conc. ext?
<headius[m]> I don't know really... it isn't a use case we have ever had to deal with
<headius[m]> if it seems simple enough to make it work I suppose we can but otherwise it might be a solution looking for a problem
<byteit101[m]> copying it would lead to different names if you rename the clone
<headius[m]> true, and at that point what is it the user really wants
<headius[m]> Why clone rather than make a new concrete extension the normal way
<headius[m]> at least I think we agree that non-ext proxy cloning should just duplicate the proxy
<headius[m]> yeah I dunno I am getting wrapped up in all the possible edge cases of cloning Java classes from Ruby
<headius[m]> updated with summary of my position... which is pretty squishy at this point
<byteit101[m]> I don't know how to override the new/initialize/jcreate/etc as per a previous question to use allocate, but I did sucessfully plumb via the IRO constructor :-)
<byteit101[m]> > Do you have any reasons why we should use the IRO constructor instead of allocate?
ur5us has quit [Ping timeout: 260 seconds]
ur5us has joined #jruby
ur5us_ has joined #jruby
ur5us has quit [Ping timeout: 264 seconds]
den_d has quit [Read error: Connection reset by peer]
den_d has joined #jruby
ur5us_ has quit [Ping timeout: 264 seconds]