sagax has quit [Read error: Connection reset by peer]
ur5us has quit [Ping timeout: 260 seconds]
nirvdrum has joined #jruby
<daveg[m]>
headius: We reverted to 9.2.6.0 and haven't had the thread explosion/locking problem yet (it's been ~10 hours, where previously happened at least once an hour). We couldn't use 9.2.7.0 because the bundler issue it in broke warbler and our jar building process. I'll add note to the issue as well.
enebo has joined #jruby
enebo has quit [Quit: Leaving.]
enebo has joined #jruby
<daveg[m]>
Let us know if you'd like us to upgrade to another version to further narrow down the issue, or if you'd like to pair/provide instruction on other things to try. Thanks again for all your help
<enebo[m]>
As a note to anyone following my work to eliminate our IR concurrency issue...
<enebo[m]>
I initially thought it was too much work to make flags specific to IRBuild and to FullInterpreterContext so that two in-flight fics would not modify the same set of flags
<enebo[m]>
This morning when auditing use of flags I realized the Instr.computeScopeFlags literally only uses getFlags() from IRScope
<enebo[m]>
So I am going to convert those all to passing in IRFlags instead of IRScope and then I think IRBuilder and any passes/fic will just pass in the flags they want which should cap that concurrency concern
<enebo[m]>
I realized that we probably could just assume toggle-true with no chance of toggle false on individual flags but that really limits possibilities on optimizations. It also is not enforced...so in one year I might accidentially set a flag back to false and then cause some weird concurrency discrpency (maybe).
sagax has joined #jruby
<headius[m]>
dave.g: yeah that's great news, we should be able to bisect to something specific
<headius[m]>
So something after 9.2.6 is a start 😀
<headius[m]>
enebo: so Instrs will populate a new flags enum
<headius[m]>
I think that will fix most things
<headius[m]>
I wish EnumSet had an immutable form
<enebo[m]>
builder and fic will have their own
<enebo[m]>
but I need to raise one more possible problem
<enebo[m]>
we compile blocks when they run enough
<enebo[m]>
we go back to hard scope and compile all of its children including that block
<enebo[m]>
if the hard scope is still executing I want to walk through the cases there
<headius[m]>
yeah that is a concern
<enebo[m]>
I have two very broad thoughts on this
<enebo[m]>
1. IRScope is not the thing to put into *Method *BlockBody
<enebo[m]>
2. If we replaced method definitions/closure definitions with when we change to fic old activations would still complete so long as all things it consumes has already been sitecached
<enebo[m]>
I call these broad because #1 is fic/ic? and #2 is not going to be 100% true
<enebo[m]>
but I was thinking old hard scope and new hard scope and downward need to be able to keep calling the old versions
<enebo[m]>
OR we do not allow compiling blocks when it has sibling blocks? Something here is nagging at me a lot
<enebo[m]>
but I want to finish this flag stuff before we tackle this and spend an hour talking about it
<enebo[m]>
Once flags is done we definitely maybe fixed something :)
<headius[m]>
#1 does seem appropriate as an IC form
<headius[m]>
and you ask the IC to give you a new IC when optimizing
<enebo[m]>
There is also a wrinkle to #1 which is IR persist + laziness
<enebo[m]>
but yeah I had same thought that one IC gives birth to the next one
<headius[m]>
LazyInterpreterContext
<enebo[m]>
we will put it in jnr-posix too
<enebo[m]>
but yeah we need to do something like that
<headius[m]>
hah
<enebo[m]>
or it is RuntimeHandle or something
<headius[m]>
yeah we did chat before about moving the triggers for optimization into IC or whatever but out of DynamicMethod descendants
<headius[m]>
since it makes more sense to have the IR responsible for itself
<enebo[m]>
anyways it removes IRScope from runtime entirely at that point which is a goal in and of itself
<headius[m]>
that would remove some of the poking from outside
<enebo[m]>
It also removes the need of links from IRScope to the ics
<enebo[m]>
I am removing all methods which reach into fic
<enebo[m]>
or I have removed them all
<headius[m]>
DynamicMethods should just have an opaque executable and we request things from it
<headius[m]>
give me a method handle to yourself
<headius[m]>
give me a bytecode version
<headius[m]>
give me your optimized self
<enebo[m]>
sure I think #1 can be done multiple ways and seems like a continuation of changes we have made
<enebo[m]>
but I do not want to consider #1 without thinking a lot about replacement of a hierachy of soft scopes with it
<headius[m]>
what's the plan for .13 then
<enebo[m]>
well unless we have an epiphany I think once flags is done we see if someone can report their problems went away and hope it is not this closure compile thing
<enebo[m]>
but for 9.3 we need to really make a change there. It seems very likely to be a source of some problems
<enebo[m]>
and interestingly it would be something people have a hard time reducing because people will reduce
<enebo[m]>
they see it happen during executing within a closure and they delete the two sibling closures next to it where one may not get called very often
<enebo[m]>
I feel like both of us could probably figure out how to exploit this with a ton of generation of methods with sequential closures where some closures are called more than others in that method but they all still compile
<enebo[m]>
maybe just a single static example def meth; 200.times { block1 { } } 100.times { block2 {} }; end or something like that
<enebo[m]>
I may be wrong about how we compile but I would guess meth is still active block1 compiles meth and both block1/2 but then block2 still has never been called on original meth activation
<enebo[m]>
if we eliminate scope in that compile then I would think we would look in wrong scope
<headius[m]>
yeah have to eval the blocks like the case we fixed in 9.2.12
<headius[m]>
so they keep being new and having to optimize
<headius[m]>
I could try to put together some synthetic breakers
<enebo[m]>
yeah please try because it would be good to see if that is more of a root then the stuff I changed or not
<headius[m]>
every test helps
<enebo[m]>
I guess it will not tell us that but I would still like to see an example
<enebo[m]>
yeah exactly
<enebo[m]>
continuing on flags moving now
<enebo[m]>
I should be done with that this afternoon
<enebo[m]>
there are some trivial things I can also move now but I think the idea they cause an issue is 0% but that might be a stretch just to complete this
<headius[m]>
ok
<enebo[m]>
There is only one oddball thing within all of flags for this work
<enebo[m]>
AliasMethod looks at scope for REQUIRES_CLASS
<headius[m]>
hsbt will release rake 3.1.1 soon
<headius[m]>
I mean psych
<headius[m]>
for the CVE fix
<headius[m]>
heh
<enebo[m]>
I thought about just adding this as a method on IRScope.requiresFrameClass()
<enebo[m]>
but the problem with it long term is classsuper and unknownsuper both set it
<enebo[m]>
It is possible long term we might eliminate those supers?
<enebo[m]>
I guess for now I can just assume once set it never goes away and put a note to revisit it
<headius[m]>
well it's a troublesome bit
<headius[m]>
pretty sure this is for module supers that have to go up the actual hierarchy
<enebo[m]>
once we switch from IRScope to FIC/IC or whatnot this goes away totally
<enebo[m]>
well how to access it not how to eliminate it
<headius[m]>
I'm not sure why it does it conditionally though, need to look
<enebo[m]>
oh for alias?
<headius[m]>
yeah
<enebo[m]>
findImplementer boolean
<enebo[m]>
so if aliased super I think
<enebo[m]>
err you super but that super is for an alias
<headius[m]>
ah right
<headius[m]>
I see it
<headius[m]>
so we do find implementer only if it REQUIRES_CLASS which is pretty much super
<headius[m]>
this should be possible to eliminate
<headius[m]>
I did a big rework of method lookup last year I think so that it has the actual implementer in hand
<headius[m]>
so it may just be an issue of getting that through call path
<headius[m]>
entry.sourceModule should be it
<enebo[m]>
I guess if you remove the logic and isolate that we have coverage and then make it work again I am happy :)
<headius[m]>
yeah dunno if I want to mess with it for .13 though
<enebo[m]>
Well I need to address this flag access to flag changes for .13
<enebo[m]>
So if we can have confidence in removing this particular use it would eliminate any weirdness with the activity
<enebo[m]>
but we should be reasonably sure it is correct
<enebo[m]>
although if you think it is correct that is great
<enebo[m]>
I do not feel most of the changes I have made is all that risky but it is a little unnerving to make a big set of changes anyways