<enebo[m]>
I guess my only concern would be order threads are asked to get killed but I think we try it and see if we see any issues...this will fix rb_inotify gem issues if it pans out
<headius[m]>
hah yeah it's pretty isn't it
<headius[m]>
this is just a relic of when we used to put Future in that map
<headius[m]>
we haven't done that since before 9k
<headius[m]>
I can't cast directly to `Map<Object, RubyThread>` because that would allow non-Thread to be added to it
<headius[m]>
note this method is deprecated anyway and should not be used
<headius[m]>
probably could delete in 9.3
subbu|away is now known as subbu
<headius[m]>
enebo: I suppose we can merge and start letting this bake
<enebo[m]>
headius: yeah
<headius[m]>
a change I considered but did not make would be to avoid the thread shutdown if we are not doing a process-level runtime shutdown, but then we'd have threads out there running against a torn-down runtime
<headius[m]>
so it seems like we always want to end "our" threads
<enebo[m]>
I complained about how secondary deps kristian added were nearly doubling the size of the distro and he confused it with me thinking I wanted another set of files that were smaller
<headius[m]>
hah yeah
<headius[m]>
poor pack200
<headius[m]>
it was a good idea that was never really integrated well
ur5us has joined #jruby
<headius[m]>
jar format should have been packed to begin with, but I suppose there's more value in having it be a normal zip
<headius[m]>
yeah I'm confused why this is failing on branches
<enebo[m]>
unless I really misunderstand something I would like you to run spec:ruby:fast locally
<headius[m]>
I could go ahead and merge pack200 and see what happens
<headius[m]>
ah sure,
<headius[m]>
I'll do that now
<headius[m]>
actually I'll just run these specs first
<enebo[m]>
I am doing the same thing but not pulling your last commit/2
<enebo[m]>
oh hmm did something in recent commits change something where the rake binstub went away?
<headius[m]>
passes locally
<headius[m]>
ah so you are seeing that
<headius[m]>
there's complicated logic for how the bundled gems get installed and I guess something's not right with the bin scripts still
<enebo[m]>
I did actually rebase and push so that is a dynamic I could have verified...it is pretty localized to things we would not trip over though
<enebo[m]>
I need to pull again ?
<headius[m]>
I'm wondering if maybe we should ditch the mvn gem bundling
<headius[m]>
pack200 branch has "Symbol type enabling moar" on it
<headius[m]>
it's based on master HEAD
<headius[m]>
but why failing
<headius[m]>
running whole spec:ruby:fast locally now
<headius[m]>
none of those failures happened locally running just those specs
<headius[m]>
mysterious
<enebo[m]>
if you run those branches locally do they fail?
ur5us has quit [Quit: Leaving]
ur5us has joined #jruby
<enebo[m]>
headius: so I have a theory but the theory only holds if I did not actually fix the issue
<enebo[m]>
spec randomizes
<enebo[m]>
so it is possible I really somehow did not fix some aspect of this and we see it based on the order
<enebo[m]>
I can actually fix this another way quickly but it is not a real fix
<enebo[m]>
This is crashing in those tests because it is walking about instance variables and it picked these up during the dump specs...they will have a null value. If I move an if against null values up higher it does not notive the ivars encoding is messed up
<enebo[m]>
So I can commit something to see if it goes away and then figure out how to run in an order to repro
<enebo[m]>
I just am having a hard time I did not fix this
<headius[m]>
hmmm that's an interesting theory
<enebo[m]>
(unless perhaps this is not from load but from dump and I only fixed it in one direction or something liek that)
shellac has quit [Quit: Computer has gone to sleep.]
<headius[m]>
I'm thinking I should merge the benign pack200 PR and see what happens
<headius[m]>
that really affects nothing relating to execution
<enebo[m]>
sure go for it
<enebo[m]>
my fix above I said is a reasonable change in its own right in that it can skip null ivars without looking at the key
<enebo[m]>
it is less work in fact
<enebo[m]>
but it was the only reason I noticed the issues with dump
<enebo[m]>
err load
<headius[m]>
ok
<headius[m]>
we need to re-audit "slow" specs and get fast actually fast again
<headius[m]>
this is too long for local sanity checking
<enebo[m]>
do I need to reinstall rake?
<enebo[m]>
how do I get that back?
<headius[m]>
that will fix it
<headius[m]>
I don't know why it's not installing the bin script
<headius[m]>
rake is one of the "bundled" but not "default" gems
<headius[m]>
the default gems are installing bin scripts fine
<enebo[m]>
just doing a sanity run on my change and I will push
<enebo[m]>
the new code requires usascii to be valid because it rightly will throw when we do stuff like force encode bogus stuff
<enebo[m]>
but the marshal.load would put crap as ascii and later get it corrected which broke the new system which was why I forced it to binary
<enebo[m]>
as such it cannot be bad so I am confused why we are seeing this now
<enebo[m]>
maybe something with dump
<enebo[m]>
although that makes no sense
<enebo[m]>
I did think of another fix to marshal.load which would be to allow a link value to be replaceable
<enebo[m]>
so you register a placeholder knowing you will replace it immediately after encoding has been decoded
<headius[m]>
no unexpected failures in local run on pack200 branch
<headius[m]>
🤷♂️
<enebo[m]>
it would be safe because we would know that nothing between those two could actually use the reigstered link
<headius[m]>
I merged pack200
shellac has joined #jruby
ur5us has quit [Ping timeout: 256 seconds]
<headius[m]>
I can merge the hard ref ruby threads PR too but we'll see how this other one lands
<headius[m]>
just eliminates weak references for non-adopted Ruby threads
<headius[m]>
and also removes all remaining reference to Future-based threads
<headius[m]>
ok on your fix
<headius[m]>
we'll see how master build lands now
<enebo[m]>
It is a reasonable commit regardless because it eliminates some symbol lookup from happening for some variables...but it is for inspecting hashy object format...which is probably never a hot path
<enebo[m]>
and it is not really saving anything we could measure :)
<headius[m]>
these failures are in inspect, so who knows, maybe it was an edge only happening in travis
<headius[m]>
I don't think specs randomize btw, they're always alphabetical
<enebo[m]>
It did get rid of a weird !(!a || b) conditional so it will not bend my mind as I demorgan it
<headius[m]>
well alphabetical by file path and then top to bottom execution
<headius[m]>
heh ok
<enebo[m]>
ok that makes no sense then
<headius[m]>
yeah
shellac has quit [Client Quit]
<headius[m]>
I'm out of ideas
<enebo[m]>
unless somehow on macos and on my linux we run differently than on linux on travis
<enebo[m]>
There is another fun bug in inspect I need to fix
<enebo[m]>
that variable it is complaining about....will not hashy inspect but will error with a decode issue
<enebo[m]>
apparently I cannot ascii8bit.cat(utf-8)
<headius[m]>
but it passed on travis twice
<enebo[m]>
yeah there is that as well
<headius[m]>
oops, check-versions script needs to remove pack200 stuff
<enebo[m]>
you know what is funny
<enebo[m]>
I enabled instance var checks to use the symbol type stuff and it complained it could not find rake :)
<enebo[m]>
I was thinking how the hell can that happen but gem list will run
<enebo[m]>
then at that moment you pointed out the problem
<enebo[m]>
on travis so I did not make it far enough to realize the binstub was gone
<headius[m]>
the binstub installs fine on travis so I think it's due to this weird conditional logic in lib/pom.rb
<headius[m]>
it looks to see if there's a cached gem file and doesn't reinstall
<headius[m]>
which means if you lost the rake script (i.e. if you updated to after my commit where I removed it) it won't get reinstalled
<headius[m]>
I just removed it from bin because we were versioning it and installing at build time
<headius[m]>
this will break for a while because some branches have bin/rake and some don't
<enebo[m]>
so it sort of notices it is there but not that some of it is not
<headius[m]>
right
<headius[m]>
it's to avoid doing the longer process of actually installing the gems when they're already there
<enebo[m]>
yeah makes sense
<headius[m]>
but rake is sort of half there across this commit
<headius[m]>
enebo: master passed
<headius[m]>
I'm going to merge in the thread PR as well
<headius[m]>
I guess we'll watch future banch builds and see what happens
<headius[m]>
I do have another PR I want to do to align 8 and 11 tests on github actions so we'll see