00:34
gordea has quit [Quit: gordea]
08:37
razetime has joined #jruby
12:51
razetime has quit [Ping timeout: 260 seconds]
13:55
razetime has joined #jruby
14:52
<
enebo[m] >
byteit101: oh interesting. Another place where we deleted a scope thinking it could never be executed again
14:52
<
byteit101[m] >
oop :)
14:53
<
enebo[m] >
I have to look at your repro but that line is interpreterContext.getInstructions which would mean we null'd it out
15:20
<
enebo[m] >
byteit101: ah...irb
15:21
<
byteit101[m] >
Yup, both in IRB
15:21
<
byteit101[m] >
one while trying to report the other
15:21
<
enebo[m] >
So we maybe notice it is an eval and decide to not keep the original around but that begs a question how id decided to JIT
15:23
<
enebo[m] >
heh...I wonder how irb works :) The closure is within a scope and the closure is added dynamically to the scope but I imagine we examine and say "no closures we can remove" or something like that
15:24
<
enebo[m] >
This is not at the top of my list today but it is curious
15:28
<
enebo[m] >
ah so running theory is we run the block n times enough to JIT and when the closure JITs we will then have to JIT all parent scopes. The parent was marked as execute once and deleted it. So when it gets to the parent it explodes
15:36
<
enebo[m] >
2022-11-28T09:35:49.270-06:00 [main] INFO MixedModeIRBlockBody : JIT failed; no full IR or no call protocol found in block: CLOSURE EVAL_CLOSURE_1[(irb):1]<startup>
15:36
<
enebo[m] >
Ok I lied.
15:36
<
enebo[m] >
9.3 detects that it has been removed and does not allow the closure to JIT
15:55
subbu has joined #jruby
16:02
<
byteit101[m] >
"If closure jits we boom." fair enough wording
16:04
<
enebo[m] >
There is followup work here but that is a reasonable check since it is not the first time we hit issues like this and it will get logged
16:05
<
enebo[m] >
It is obvious what is happening but not obvious how it got into the state.
17:16
subbu has quit [Ping timeout: 260 seconds]
17:17
<
headius >
My flight is delayed tonight, working on talk and catching up in here
17:19
<
headius >
Fix looks nice and simple
17:20
<
enebo[m] >
so mocha fails 4 tests and it looked like kwargs but it is in fact something we changed in generating backtraces
17:20
<
enebo[m] >
which is almost as unfun as solving kwargs bugs :)
17:21
<
enebo[m] >
but we did change backtraces for 9.4 and we pass on 9.3
17:22
<
headius >
Something we changed in back traces is causing an error?
17:22
<
enebo[m] >
the testing here gathers a bunch of blocks and then uses a name to define_method them
17:23
<
enebo[m] >
our backtrace finds the name of that defined method instead of the test method it is called from
17:23
<
enebo[m] >
I have not fully hit the reason here but MRI will give the test name as the first element and we will not
17:23
<
enebo[m] >
but I rankle when I see JRuby-specific stuff in code :)
17:24
<
enebo[m] >
They strip org.jruby from backtrace elements
17:24
<
headius >
So it is using caller or something?
17:24
<
enebo[m] >
that is not the issue but it makes me wonder what else could be happening
17:24
<
enebo[m] >
def self.current; new(caller); end
17:25
<
enebo[m] >
I have not distilled this enough to know what is wrong but I am happy it was not kwargs
17:25
<
headius >
There was one change I remember to make arity errors show up in the method that was being called but I don't think that should affect this
17:26
<
enebo[m] >
the code we shipped has veered off so long ago
17:26
<
headius >
Have you isolated a test case?
17:26
<
enebo[m] >
funny I ripped out the test and made a mock and it was fine
17:26
<
enebo[m] >
So I have to actually use their testing infra to deduce what the testing code is doing
17:27
<
enebo[m] >
Actually this is icky too. It will make test/unit or minitest runnable thing and then run it
17:28
<
enebo[m] >
so each test is put into a new class which represents the framework it uses but derives from Mocha::TestCase then runs that
17:28
<
enebo[m] >
For all I know this is some artifact not in mocha at all munging out backtraces
17:29
subbu has joined #jruby
17:29
<
enebo[m] >
These are the sort of bugs I find pretty unimportant in the grand scheme of .0 but it is a popular gem and we fail 4 tests
17:29
<
headius >
If it worked before it's probably going to be an easy fix
17:30
<
enebo[m] >
oh but it did and didn't :)
17:30
<
enebo[m] >
It worked before but these are tests only run for 2.7+
17:31
<
enebo[m] >
So they may be something we never worked with
17:31
<
enebo[m] >
9.3 just happens to avoid hitting these because it is not about warning on 2.7+ kwargs deprecating behavior
17:34
<
enebo[m] >
The issue
17:51
<
headius >
enebo: trying to run railsbench with indy mode and I hit that UndefinedValue thing
17:52
<
enebo[m] >
ok. try with --dev just to see if it goes away
17:52
<
headius >
it seems to work without indy
17:52
<
enebo[m] >
threshold=0 too?
17:52
<
headius >
all I did was run without indy and then run with indy
17:53
<
enebo[m] >
I am curious if this is fixed arity kwargs issue or something with indy I was not aware of
17:53
<
enebo[m] >
I am sure I have run various things with indy enabled but I guess it depends on what
17:53
<
headius >
we have several suites running both ways too
17:54
<
enebo[m] >
try threshold=0 and make sure it is not a generic issue
17:54
<
enebo[m] >
I know there is something from the mechanize bug
17:54
<
enebo[m] >
err whatever that web-driven project is
17:55
<
headius >
seems ok through first few iters
17:55
<
headius >
you added something to debug this no?
17:56
<
enebo[m] >
I have an operator which only in startup interp will print out live operand values as a debug op
17:56
<
enebo[m] >
It should not be too hard to compile those
17:57
<
enebo[m] >
I mean cool in the sense it might be much less convoluted than the other report
17:57
<
enebo[m] >
this looks similar
17:58
<
enebo[m] >
It was not in initialize_copy but this clearly is some recv instr grabbing a value %undefined when it should not be doing that (or something similar)
17:58
<
enebo[m] >
I am going to look at what that is in active_support
17:58
<
enebo[m] >
dup is fairly primal. hopefully it is not some stuff @ivar holding it
17:59
<
enebo[m] >
activesupport 6?
17:59
<
headius >
6.0.6 yes
17:59
<
enebo[m] >
ok yeah just wondering if this is kwargs how that works out
18:00
<
enebo[m] >
self.__callbacks = __callbacks.merge(name.to_sym => callbacks)
18:01
<
headius >
yeah it is fairly simple
18:04
<
headius >
merge did change in the last week or two
18:05
<
enebo[m] >
ok well it is dup'ing itself which sort of sucks
18:06
<
enebo[m] >
That means __callbacks is %undefined or something happening in dup is undefined
18:06
<
enebo[m] >
which is possible
18:08
<
headius >
I should be able to set a breakpoint on this undefined error and see what's on stack
18:08
<
enebo[m] >
So I would say __callbacks is %undefined
18:08
<
enebo[m] >
I do not see how __callbacks is ever initially set
18:09
<
headius >
it has a default empty hash
18:09
<
enebo[m] >
class_attribute :__callbacks, instance_writer: false, default: {}
18:09
<
enebo[m] >
err I missed default
18:09
<
enebo[m] >
so I wonder if this method is broken with kwargs
18:12
<
headius >
it's something in the dup
18:12
<
enebo[m] >
so the hash is ok up until that point?
18:12
<
headius >
at the stack level of merge both hashes are fine
18:13
<
headius >
# Replaces Java version for better caching
18:13
<
headius >
initialize_copy(original)
18:13
<
headius >
def initialize_dup(original)
18:14
<
headius >
that's our pure-Ruby initialize_dup
18:14
<
headius >
by the time we get in there arg0 is %undefined
18:15
<
enebo[m] >
sites(context).initialize_dup.call(context, dup, dup, this);
18:15
<
enebo[m] >
so it is fine right there?
18:16
<
enebo[m] >
heh...well that is alarming
18:16
<
headius >
all the way up until the jitted initialize_dup body is called
18:16
<
enebo[m] >
ah the JITTed one
18:17
<
enebo[m] >
I wonder if there is something very basic wrong with 1-arity JIT
18:17
<
enebo[m] >
I would think normal JIT should see it
18:17
<
enebo[m] >
ah but this is happening with indy
18:18
<
headius >
yeah wherever the call is setting up these incoming args it loses the hash
18:22
<
headius >
INVOKESTATIC org/jruby/ir/runtime/IRRuntimeHelpers.receiveSpecificArityKeywords (Lorg/jruby/runtime/ThreadContext;Lorg/jruby/parser/StaticScope;Lorg/jruby/runtime/builtin/IRubyObject;)Lorg/jruby/runtime/builtin/IRubyObject;
18:22
<
headius >
INVOKESTATIC org/jruby/ir/runtime/IRRuntimeHelpers.undefined ()Lorg/jruby/runtime/builtin/IRubyObject;
18:22
<
headius >
probably something in here
18:23
<
headius >
there's not much more in the bytecode
18:23
<
headius >
not sure why this would only affect indy
18:24
<
enebo[m] >
well it might not only affect indy
18:24
<
enebo[m] >
ReciveKeywordsInstr in JVMVisitor is my bet on an issue with specificArity
18:25
<
headius >
return hash.isEmpty() ? UNDEFINED : hash.dupFast(context);
18:25
<
enebo[m] >
where is that?
18:25
<
headius >
receiverSpecificArityKeywords
18:27
<
headius >
looks like call info is getting set up as CALL_KEYWORD before this initialize_dup call
18:27
<
enebo[m] >
I believe the notion of this is that if you receive an empty hash it is not really a valid argument but an empty kwargs
18:28
<
enebo[m] >
which weirdly can happen to any call like 'Array.size(**hash)
18:28
<
enebo[m] >
so it ends up as 0 arity call
18:28
<
enebo[m] >
although we handle Ruby -> Ruby where it does not pass it
18:28
<
enebo[m] >
but native call to Ruby maybe?
18:29
<
enebo[m] >
I have always felt this is gross but with all that said I am a bit confused why UNDEFINED is there
18:29
<
headius >
yeah the callinfo state is not set up right perhaps
18:29
<
enebo[m] >
either it should be EMPTY kwargs in callInfo or that undefined is not right there
18:30
<
enebo[m] >
The big problem with this method is it only happens on JIT and only when fixed arity
18:30
<
enebo[m] >
so it is not uncommon but also not as predictably called...with that said I feel this has to happen a ton
18:31
<
enebo[m] >
which means how we call it is the strange thing
18:31
<
enebo[m] >
You can just return hash.dupFast() as that last return and see if it stop exploding
18:31
<
headius >
yeah probably will
18:31
<
enebo[m] >
I am curious enough to see what breaks
18:31
<
headius >
trying it
18:32
<
enebo[m] >
for fixed arity this would only make sense if we eliminate a parameter from being valid
18:32
<
enebo[m] >
but recieve instrs are not littered with checks
18:32
<
headius >
it appears to be running
18:33
<
headius >
yeah it works now
18:33
<
headius >
non-indy run may be going through a different path here that isn't specific arity
18:33
<
enebo[m] >
One issue with this when I was working with it was I started from interp version of this method and tried to adapt it to specific arity
18:34
<
headius >
I see nothing in here that should change due to indy
18:34
<
enebo[m] >
but people did have a similar report with kwargs on that web-automation package
18:34
<
enebo[m] >
with just JIT
18:34
<
headius >
good news is that indy mode is avout 40% faster than non-indy on this silly bench
18:35
<
enebo[m] >
I am wondering if any pass of {} as last specific param is part of the cause
18:35
<
enebo[m] >
there may be another piece like some callInfo state setup?
18:36
<
enebo[m] >
so to hit that path it HAS to be CALL_KEYWORD meaning callsite sending set that value
18:36
<
enebo[m] >
any @JRubyMethod marked keywords will not flush callInfo
18:36
<
headius >
merge is just rest
18:37
<
enebo[m] >
So native calling Ruby could erroneously get it from a native keywords marked method which did not reset it
18:37
<
headius >
that goes into native dup which eventually calls initialize_dup in ruby
18:37
<
enebo[m] >
Some methods do want resetting and others want to forward
18:37
<
enebo[m] >
dup and initialize_copy are normal
18:38
<
enebo[m] >
Actually where is the main dup defined?
18:38
<
enebo[m] >
well for Hash it is there and no big deal
18:39
<
enebo[m] >
initializerdup is no big thing either
18:39
<
headius >
RubyBasicObject.dup eventually does the initialize_dup call
18:39
<
headius >
merge calls that
18:39
<
headius >
from Java
18:39
<
enebo[m] >
yeah but there are fixed arity no special stuff
18:40
<
enebo[m] >
and since they are dyn dispatch they should be wiping out callInfo
18:40
<
enebo[m] >
hmmm I wonder if AS overrides these
18:41
<
headius >
yjit seems to be faster on this
18:41
<
headius >
3.1 without yjit is about equivalent
18:42
<
enebo[m] >
push that change out to a PR and see what blows up
18:42
<
headius >
oh wait nevermind
18:43
<
headius >
we are much faster with parallel GC again
18:43
<
enebo[m] >
even with 19?
18:43
<
headius >
it gets down faster than yjit a few iters but then regresses
18:43
<
headius >
I'll try 19
18:43
<
headius >
regression might be my machine throttling too
18:44
<
enebo[m] >
I think I disabled speedstep
18:44
<
enebo[m] >
orwhatever that thing is
18:44
<
headius >
yeah that would be helpful
19:14
<
headius >
enebo: ok I will push this in PR
19:18
<
enebo[m] >
cool...eating lunch
19:33
subbu has quit [Quit: Leaving]
20:20
subbu has joined #jruby
20:41
sagax has joined #jruby
23:35
subbu has quit [Ping timeout: 265 seconds]
23:37
subbu has joined #jruby
23:59
subbu has quit [Ping timeout: 260 seconds]