<chrisseaton[m]1>
I also think FFI is generally better, and FFI in terms of FFI, or FFI in terms of Fiddle would be better - that PR is just about improving Fiddle as it is
<headius[m]>
well I mean our fiddle is based on FFI but we're the only ones maintaining it
<headius[m]>
yeah I get it
<headius[m]>
just trying to get a handle on what will be necessary in JRuby's fiddle
<chrisseaton[m]1>
The thing is Fiddle ships in MRI, would they also ship FFI? I don't know
<headius[m]>
I wish they would have years ago but there were dubious reasons not to
<chrisseaton[m]1>
JRuby's Fiddle seems to have some missing things in my experience (not a criticism! it's under-tested and we all have bugs)
<headius[m]>
so Aaron made Fiddle
<headius[m]>
oh it's missing tons of stuff
<headius[m]>
I think it was half done when wmeissner checked out
<chrisseaton[m]1>
I've done quite a deep dive into Fiddle here - I may look at some options for how to make it portable going forward and I'll involve you
<headius[m]>
yeah thanks... it may make sense for us to do a native fiddle on top of JNR but I've never gotten a good look at what that native part really is
<headius[m]>
if it's small enough and we can share the rest, I'm on board
<headius[m]>
ugh come on azure
carla[m] has joined #jruby
ur5us has quit [Ping timeout: 260 seconds]
ur5us has joined #jruby
_whitelogger has joined #jruby
<headius[m]>
I have every issue we discussed today merged, but have to work out some build issue
<headius[m]>
enebo: so in the process of cherry-picking those changes across I also remarked the issues and PRs for 9.2.12
<headius[m]>
if I could have just cloned the issues that would maybe have been preferably but this seemed the best way to have an accurate issue list
<enebo[m]>
headius: yeah I did that for the one I did too
<headius[m]>
ok
<enebo[m]>
but 9.3 is not out so all 9.2 fixes will be in 9.3
<headius[m]>
kares: when you get your thing picked over make sure to remark it from 9.3 to 9.2.12 so it will be in release notes
<headius[m]>
right, that's what I figured
<headius[m]>
I really really wish GH allowed multiple milestones per issue
<headius[m]>
that's a big one I miss from JIRA
<headius[m]>
like, doesn't anyone on GH have multiple maintenance branches?
dopplerg- has joined #jruby
dopplergange has quit [*.net *.split]
<headius[m]>
jmalves: I updated your bug and I'm looking into the AIOOB but I have not seen the NPE on 9.2.12 so far
<headius[m]>
I'm letting it run in the background
ur5us has joined #jruby
<headius[m]>
enebo: this OOB is weird
<headius[m]>
I'm looking at the instruction that breaks and it's part of an instr[] that does not push a scope
<headius[m]>
but it has ClosureLocalVariable as its result
<headius[m]>
oh wait I may have a theory now
<headius[m]>
if it's going down the direct path with a non-optimized IR I think this would happen
<headius[m]>
because it's expecting code around it to have pushed something
<headius[m]>
seeing ClosureLocalVariable with no scope push was weird to me but I guess that's what it is when we are doing the scope management separately
<headius[m]>
ah yeah
<headius[m]>
this is likely a concurrency issue with flags again
<headius[m]>
InterpreterContext lazily acquires hasCallProtocol from the scope
<enebo[m]>
I have some times wondered if we would do better to instead of just adjust depth make a new type to make it obvious it is a variable which is assuming a depth/scope has been removed
<headius[m]>
so if it got optimized in between then you'd have a non-ACP scope inside an IC that says it has ACP
<headius[m]>
see InterpreterContext.setEngine, which calls retrieveFlags
<enebo[m]>
It would only be in name only as the field values would be the same
<headius[m]>
yeah that would be helpful
<headius[m]>
I think we can fix the error message without changing anything else
<headius[m]>
the static scope is there
<headius[m]>
"tried to read xxx from scope yyy"
<enebo[m]>
It would allow us as a glance to notice the mismatch although I have thought it was not needed because all AIIOBE in scopes is from this mismatch
<enebo[m]>
are
<enebo[m]>
man I am losing english as a skill
<headius[m]>
do you know why retrieveFlags is lazy in IC?
<enebo[m]>
blame commit is far from clear
<headius[m]>
by the time we construct IC, we should be done with flags
<headius[m]>
I mean, for that IC anyway
<headius[m]>
any further changes should not be seen by IC
<enebo[m]>
Ok looking I think one issue was we used to just do this in the constructor but at some point moved it into getEngine
<headius[m]>
probably to fix some other concurrency issue
<headius[m]>
I'll undo that fix now if I move it back
<enebo[m]>
honestly though...getEngine is not synchronized and does lots of other field sets
<headius[m]>
yeah
<headius[m]>
ah I see we do call setEngine in one constructor
<headius[m]>
but not in the other
<enebo[m]>
So in essence I think the problem we we initialize a lot in getEngine
<enebo[m]>
That leads in two directions
<enebo[m]>
either make sure two threads cannot call this or perhaps not make this lazy anymore or at least not from something with competing threads
<enebo[m]>
I know getEngine is used to stand up instrs from ir persistence too
<headius[m]>
IC should be immutable
<enebo[m]>
Laziness is a horrible horrible really nice feature :)
<headius[m]>
indeed
<enebo[m]>
yeah that is probably true
<headius[m]>
I'm trying to move the field sets into constructors
<headius[m]>
if they're changing after IC is constructed that's a bug elsewhere in the workflow
<enebo[m]>
We do not want to perform a bunch of work in places where we never interpret or compile
<enebo[m]>
IR persistence is an obvious first example
<enebo[m]>
We could obviously extract the starter info it provides like instrs and then provide it to an IC and then make the IC immutable but then something higher up will not be
<enebo[m]>
but IC and FIC should have a finished state and nothing should change after that
<headius[m]>
yeah
<enebo[m]>
the implication is it may have a start state that is not all in a constructor
<headius[m]>
I will make this stuff final and try to ensure the IC creation is atomic with the state of scope
<headius[m]>
just moving to constructor did not help yet
<enebo[m]>
A fast first step may be to just audit what asks for getEngine and getInstructions and make them safe
<enebo[m]>
I mostly suggest that to prove the AIIOB is only from this
<headius[m]>
mmm yeah maybe sync them as a first pass
<headius[m]>
I will play with this for a bit... I have those fields final, and the construction of IC is sync on scope, but the flag updates are still happening in place
<enebo[m]>
One original goal was different distinct types which would do their work and when done they would replace or set the IC
<headius[m]>
we should consider making IRScope have a single mutable field that's an immutable tuple of its state
<enebo[m]>
but once we started wanting to make some stuff lazy then we have multiple IC types and they both have lazy aspects
<headius[m]>
so it switches over atomically and we only have one thing to read
<headius[m]>
so much mutation in place here I'm not sure I can make it all safe
<enebo[m]>
yeah that was how it was originally but then we realized preparing instrs you may not use costs startup
<headius[m]>
well we don't have to prepare any more than we do now
<enebo[m]>
So the original work thought about this crossover but it has gotten more complicated over time
<headius[m]>
but when we do we swap out that state object altogether
<enebo[m]>
anyways I am going back to warning on use/def..I am close to being done
<headius[m]>
ok
<headius[m]>
I'm trying not to make this a big concurrency cleanup but it's overdue