<jswenson[m]>
👋 I know its a little late in the process already, but I’m playing with 9.3.0.0 (using the most recent commit on master) with Looker. I’ve run into two issues so far. I’m currently trying to reproduce and file tickets for them. Just thought I’d give you a heads up.
<headius>
Hey great, got something you can file yet?
<headius>
We fully expect issues after this long a development cycle but we know people tend not to try release candidates. The plan is just to get the .0 release out and then work on bug reports for a few weeks
<jswenson[m]>
First one is pretty simple, I haven’t reproduced it outside of the rest of our system, but I THINK I can file it -- second one I don’t understand yet. was able to work around it though and boot on 9.3.0.0
<enebo[m]>
jswenson: what was the nature of the first one?
<enebo[m]>
hmm looks like we removed helper methods for class/module references
<enebo[m]>
In the past that was us doing a def CompanionObjectName; ...
<headius>
I guess we need to pull the trigger on getting some Kotlin tests into the repo
<jswenson[m]>
might be that it needs a @JvmStatic annotation on it or something
<headius>
jswenson: if you can provide a trivial example or repo it should be easy to investigate
<jswenson[m]>
so very possible that we’re doing something that just happened to work
<jswenson[m]>
yeah will try to do that.
<headius>
I do remember that annotation making it easier for us because the name becomes predictable perhaps?
<headius>
it has been a bit since I looked at the Kotlin enhancements
<enebo[m]>
personally the :: feels more right
<jswenson[m]>
oh I supposed that would have been on the method
<enebo[m]>
not that we should be breaking stuff (although we do consider this a major release but I don't recall this as an intentional change)
<enebo[m]>
kares: Any thoughts on #6845
<enebo[m]>
jswenson: we might put a fix into 9.3.1.0 and we expect a quick point release since no doubt we will get some other reports
<enebo[m]>
unless this was intentional :)
<headius>
we should get some kotlin into core/src/test and at least try basic integration examples from JUnit
<enebo[m]>
com.foo.Constant1.Constant2 I think was some convenience in JI but it is a bit weird from a Ruby syntax perspective
<enebo[m]>
I am trying to think of what core Java API has this need? Iterator?
<jswenson[m]>
We’re spending a little time “hacking” this week and i figured I’d play with what ya’ll did in https://github.com/jruby/jruby/pull/6590 and try out 9.3 at the same time
<jswenson[m]>
the other one I’m not quite sure what is happening, but we’re doing some dynamic work and depending on how something is called seems to be yielding a different stacktrace when use use the `caller` method
<jswenson[m]>
there’s a fair bit to untangle that I’ve never explored yet, so I’m not quite ready to file that one yet.
<jswenson[m]>
looks like we’re trying to work around something that changed with what we were doing something in 9.1 that may have changed again.
<headius>
hmm that sounds interesting
<headius>
using caller output to do dynamic code stuff?
<jswenson[m]>
Yeah
<jswenson[m]>
Its only used in dev
<jswenson[m]>
or in build
<jswenson[m]>
we’re trying to reject certain frames of caller.
<headius>
if you can identify the traces that are different I can probably figure it out... the stack trace management is mostly my stuff
<enebo[m]>
binding of caller is something we don't support or do we?
<enebo[m]>
It has been such a long time I thought about that
<enebo[m]>
whoops updated gist to include after
<enebo[m]>
This took a bit more work than I expected too..the ir pass scheduler only allows a pass to run once which is a flaw in the design. Otherwise I had to change call to only be a barrier if call can be eval or has a literal closure attached
<headius>
binding of caller only works in interpreter mode because it requires deopting an arbitrary number of calls up the stack
<headius>
it's not possible for us to fully support without direct JVM stack access, and even then it would be really hard
<enebo[m]>
headius: yeah I knew why it wouldn't work for all cases I was unsure if we supported it at all
<enebo[m]>
but interp will work is the answer
<headius>
yeah
<enebo[m]>
So I am trying to nail down what other conditions we need to consider
<enebo[m]>
binding, eval, literal block
<headius>
so many line numbers
<enebo[m]>
for JIT it does not matter
<headius>
not really a concern in bytecode but I think it does increase bytecode size a little
<enebo[m]>
full interp it does but we need a cheap mechanism to cull them
<enebo[m]>
I thought we kept track and only emitted last one
<enebo[m]>
oh maybe not
<headius>
hah I tried to test at command line but of course it's all line 1 then
<enebo[m]>
We do it for IRBuild but that is because it is free
<enebo[m]>
Perhaps we can do it in LocalOpt with a little extra state. I would hate to make a pass for that
<enebo[m]>
note that today it is unlikely you will see this very much
<headius>
I collapse consecutive line numbers that are the same number but not sure how I would detect this
<enebo[m]>
with my changes to localopt pass it will become more likely
<headius>
yeah this is a similar case, a = 1, b = 2, c = a + b etc
<enebo[m]>
I believe I can track this during optimizing instrs
<headius>
stuff people don't do
<enebo[m]>
yeah that is part of the problem with my example
<enebo[m]>
no one does that
<headius>
jswenson: reject for reporting reasons?
<headius>
like rspec mining stack trace to only print the relevant part
<enebo[m]>
but I was annoyed to not see the literals propagate
<headius>
mmm
<headius>
this code boils down to load 1, load 2, add, load 3, add (there's a d = c + 3 I did not show)
<headius>
so literals are propagating but it is honoring override of + and friends
<enebo[m]>
it also killed the need for the copy which is something the JIT will kill off for us but I am liking the idea of running localopt multiple times
<enebo[m]>
anyways...if you think of other boundaries where a call can access the parent's local variables
<enebo[m]>
reducing bytecode was a part of this but also thinking about inlining will make running multiple localopt passes pay off
<jswenson[m]>
headius: it looks like that “cleanup” is not actually doing anything in this case. I think we were removing frames with negative line numbers. But none of those exist in this stack. Instead it looks like there is a missing / additional frame in some cases.
<jswenson[m]>
yeah this was back in 2017 when we moved from 9.1.5.0 -> 9.1.7.0
<headius>
oh yeah
<jswenson[m]>
good to know we can probably clean that up now -- its probably been silently doing nothing for years
<headius>
Yeah once we get this fixed I'd love to have you just remove your stack scrubbing and we can fix anything else that's not right
<headius>
I am disappointed this is not removing the extra frame. I'm sure I tested it but perhaps I tweaked something at the last minute. Serves me right for not coming up with a test
<jswenson[m]>
what surprises me is that the get and the patch seem to work differently
<jswenson[m]>
one with and and one without that extra frame
<headius>
Is one getting called with a different number of arguments? The only way the extra frame could get in there is if it were called with a variable number of arguments
<headius>
If you call a method that takes two arguments and pass two arguments we don't go through this extra frame
<jswenson[m]>
they do look identical from what I’m looking at here, but I also wouldn’t be surprised if there is another layers of dynamic metaprogramming coming from sinatra
<jswenson[m]>
or other hooks in our system for routing
<jswenson[m]>
they both are `verb ‘/path/ do ; ... ; end`
<headius>
You can call java.lang.Thread.dumpStack to see if the extra frame shows up in the raw trace. I need to fix printing the raw trace from JRuby
<headius>
I think I just botched the frame name comparison in that code I linked
<jswenson[m]>
yeah that looks right -- `patch` the one with the extra frame seems to have a VARARGS in there
<jswenson[m]>
really messed up and sanitized, but something like that
<headius>
I have to run an errand but take a look at the code I linked and you might see the problem. I probably missed a dollar sign or something when comparing the frames
<jswenson[m]>
yeah seems like you’ve got a good handle on it. Will try to come up with a simple repro for the other bug.