<headius> Welcome back
<puritylake[m]> How are you headius?
<headius> Doing well! Back to serious work tomorrow
JasonLunn[m] has quit [*.net *.split]
distant[m] has quit [*.net *.split]
Freaky has quit [*.net *.split]
Freaky has joined #jruby
Freaky has joined #jruby
Freaky has quit [Signing in (Freaky)]
distant[m] has joined #jruby
JasonLunn[m] has joined #jruby
<puritylake[m]> Been on holiday?
<puritylake[m]> I’ll be back to work on JRuby either today or tomorrow. Possibly squashing bugs to start out with
<headius> Yeah catching up some family trips we missed the last few years
<puritylake[m]> Ah nice, sounds like a good time, I had noticed the chat was fairly empty
<headius> Yeah time to get back in there!
<puritylake[m]> Enebo still kicking around, or did he also take holiday time?
<headius> I think he took some breaks here and there but not sure of his status
<enebo[m]> I have been working but just mostly in silent mode
<enebo[m]> I landed new kwargs using out of band callInfo on TC
<headius> nice
<headius> I am catching up issues to get back into the swing of things
<enebo[m]> I definitely think we may want to consider pushing some kwarg call paths so we can get away from that method
<enebo[m]> k77chi just reported a potentially decent thing with timeout
<headius> I agree, it needs to happen... for 9.4 or after?
<enebo[m]> He said in last sentence we will get more green
<headius> yeah I saw that
<headius> have not looked at the issue yet
<enebo[m]> I think it is to be determined on kwargs. I should probably even write something up and what is good and bad on how it works now
<enebo[m]> puritylake: on other thing I was working on was implementing grisu2 for float/double to string
<enebo[m]> It is implemented but some calculation is drastically wrong so I need to debug it
<headius> performance wise it will have an impact but we already go to TC for a lot of stuff
<headius> the other aspect is having state bugs if some logic fails to update TC properly
<puritylake[m]> Ah interesting, I was kinda dreading getting those float/double to string code in lol
<headius> passing the info along call path is clearly the way forward, it's just a lot of duplicated signatures to update
<enebo[m]> yeah I mean it hits a field and reads it and then resets it. So math benchmarks? maybe
<headius> yeah it is just memory reads and writes
<headius> on same thread
<headius> we know there's a cost updating frames and such but it is small if there's any other heavy lifting (e.g. allocation) going on
<enebo[m]> seriously I think the bigger problem is that it is really brittle
<headius> and until we can splat out kwargs on both ends and avoid the hash object it will probably be miniscule
<headius> hash is huge overhead in comparison
<enebo[m]> If you inject any "ruby" call between setting and reading that other call overwrites it
<headius> yeah so it has to happen purely in the substrate between caller and callee
<enebo[m]> So a few places we capture the value knowing it is likely to do some coercion (e.g. to_hash) and then consume it or reset the value
<headius> that is the kind of state change but I was referring to
<headius> bug
<enebo[m]> which is a big reason it should just be a param down the call path
<headius> hah yeah scary
<headius> make it a stack
<headius> loljk
<enebo[m]> I also conditionally added "forward=true" which preserves consuming it in native methods which plan on forwarding the callinfo to another method like Class#new is really just forwarding to initialize
<enebo[m]> In truth all methods have to handle some_method(real_arg, **{}) even when they don't support kwargs
<headius> ugh I can never remember some IR class names
<enebo[m]> in Ruby to Ruby calls this is all handled by instructions (and ruby callsites just omit passing empty kwargs
<headius> set or put or assign
<headius> PutConstant today
<enebo[m]> I would be happy if they were literally the name of the class
<enebo[m]> I am not attached to pretty names
<headius> so my opinion on adding the call paths is that the perf gain is not a driving factor until we can propagate kwargs everywhere, which is probably too big for 9.4
<headius> so it would center around having more robust kwarg state handling
<enebo[m]> yeah I am in the same boat
<enebo[m]> I think it is the right step but it is a massive undertaking
<enebo[m]> I think we limp this design along and then get 9.4 out
<puritylake[m]> Need any help on my side? I’m happy to go bug squashing until 9.4 releases if needs be
<enebo[m]> puritylake: any fixes of anything not passing on master/9.4 is a big help
<enebo[m]> for the grisu2 impl I noticed there are other Java impls so if I cannot figure out which math is wrong I will just run those and see how my math differs from theirs.
<enebo[m]> This is all annoying because all the impls come from the same paper but they all have different licenses
<headius> enebo: you have any issues where generated pom.xml have element ordering changes?
<enebo[m]> ummm
<headius> I know we have seen this from time to time but I'm running on Java 17 locally and they are always showing as dirty after a build now
<enebo[m]> I don't really grok that. Perhaps unrelated but I can never load pom.rb in idea for core unless I delete it and depend on the pom.xml instead
<headius> yeah that is an idea bug
<enebo[m]> hahah
<headius> pretty sure this is just polyglot not using a specific order and letting hash order happen
<enebo[m]> I have not noticed that but I don't really regenerate those any more than version numbers so I may not notice it
<headius> they just generate on build so it is annoying
<enebo[m]> updating VERSION will regenerate but they are already changing the files
<enebo[m]> looks like some hash weirdness
<headius> add it to the queue of things to fix
<enebo[m]> don't we have an option to disable random hashing?
<headius> I don't know if it would matter... I think this is just differences in how e.g. Hashtable hash in newer versions of Java so iteration encounters keys in a new order
<headius> and polyglot just uses the order the hash/map gives
<headius> it might even be fixed in newer polyglot, have not looked into that
<enebo[m]> so we don't control the hash used either
<enebo[m]> but this only fires if the .rb changes?
<enebo[m]> Or you said for all builds but I wonder why?
<enebo[m]> I guess maybe to avoid just depending on dates
<headius> it generates every time
<headius> I think the maven builds still depend on the xml so polyglot always generates it
<headius> maven itself depends on the xml I mean
<headius> there's no direct hand-off of the model from polyglot to maven
<enebo[m]> yeah so no date checks to know if it needs it probably because that is not 100%
<enebo[m]> well it is 100% unless you have a weird environment
<headius> yeah
<enebo[m]> anyways I guess that is not really a big issue since 17 is quite a ways out from being a lowest dev build but annoying if you accidentally start commiting that sutff :)
<enebo[m]> going to go for a run this morning...70F!
<enebo[m]> It will be hot as balls the rest of the week
<headius> yeah storms cooled things down a bit
<headius> I will keep reviewing bugs, fixing some, and catch up
subbu has joined #jruby
<headius> enebo: this is clearly an IR bug... the fast path for a rescue body with no return logic is to copy the exception into the return value for the begin/rescue, but that clearly is wrong when it should go into a specific variable:
<headius> we could try nulling it out in that case but I'm not sure what else below might be expecting the exception to be in `rv`
<headius> removing the Copy does not work... breaks something in RG during boot so it is probably not the right fix
<headius> this is very old code so this probably has always been broken
<enebo[m]> yeah
<enebo[m]> I will take a look too. I saw this come in but I was trying to finish off landing kwargs branch
<enebo[m]> hahah may
<enebo[m]> well perhaps it came in earlier than that
<headius> It's probably simple but my knowledge of the rescue building logic is a little fuzzy
<enebo[m]> oh yeah this was reported like a few days after our last release
<headius> Notice it only affects the case where you have an empty rescue and you are also assigning the exception to a variable
<headius> Perhaps the compile of the body should not be returning that variable when there's nothing else in the body?
<enebo[m]> yeah this is interesting because I think you could argue it is not empty
<enebo[m]> So that might be the fix
<headius> Any assignment done for the exception variable should not be considered part of the body, at least for return value purposes
<headius> The result of assigning that variable should just be dropped
<enebo[m]> The assignment to _e should perhaps reflect it is not an empty body assuming that is simple
<headius> I think you're right
<headius> It sees that there's a variable assignment and treats it as a non-empty body with the result of the variable assignment as the return value
<headius> I'm not sure how to fix that since the body probably does have the variable assignment as a normal instruction. It just shouldn't be considered the return value of the body
<enebo[m]> so opposite of what I said. It is empty but it thinks the assignment is part of the body
<headius> Right
<enebo[m]> in my mind it is but the => assignment is .... hmm
<enebo[m]> This is a generic thing now in Ruby too
<headius> So it is not really a bug in the rescue logic, it is a bug in determining whether the body has a returnable result
<enebo[m]> what does p("foo" => e) do?
<enebo[m]> I wonder if that even returns "foo" or not
<enebo[m]> lol
<enebo[m]> hmm
<enebo[m]> It thinks it is a hash now :)
<headius> I think this just needs to recognize that the body starts after the assignments and check whether that is empty or not
<enebo[m]> This may also be a bit academic since this is a Ruby 2.6 bug and earlier and the generic form of rhs assignment is a newer feature
<headius> The variable still needs to be assigned but it should not be considered the last instruction in the body for return purposes
<enebo[m]> Now I want to know how that works though
<enebo[m]> It will be the same as the more generalized feature
<headius> It's just an assignment
<enebo[m]> so as an expression it returns the value
<enebo[m]> wow intellij crashed
<headius> Well done
<headius> The assignment is really part of the preamble to the body
<headius> Like assigning local variables from method parameters
<headius> The assignment should not be considered part of the expression that is the rescue body
<headius> Yeah nothing to indicate the assignment is not part of the body
<enebo[m]> So arraynode is the exceptions nodes to match and the local asgn is the body
<enebo[m]> but yeah There is nothing here to differentiate between StandardError => _e and StandardError\n_e = $!
<enebo[m]> The second case would have the return we have
<enebo[m]> yeah I am going to look at MRI. We clearly need something in the AST since we cannot read this node and differentiate between the two
<headius> A simple fix might be to always compile and empty body to have a nil result
<enebo[m]> but how do we know it is empty?
<enebo[m]> my above two snippets will generate the same AST
<enebo[m]> well I think so anyways...let me make sure
<headius> Oh right AST just treats it like any other assignment
<headius> Yeah MRI must be differentiating this somehow
<enebo[m]> yeah the only difference in the AST with that change is the line number of the assign
<enebo[m]> That won't cut it so I think we probably need to change our tree a little
<enebo[m]> but I believe our IR defaults to nil so it will be nil on no body
<enebo[m]> opt_rescue: k_rescue exc_list exc_var then... (full message at https://libera.ems.host/_matrix/media/r0/download/libera.chat/04f9de1638577234d154ce57ba6a43bbb147e2ba)
subbu has quit [Quit: Leaving]
<enebo[m]> come back
<enebo[m]> So if $3 is present this will add errinfo?? to $3 and then appends it onto compstmt
<enebo[m]> which is the body
<enebo[m]> hmm
<enebo[m]> So compstmt in their tree ends up being nil and ours is null. So they get '_e = $!; nil' and we just get '_e = $!'
<enebo[m]> I think I can fix this with makeNullnil($5) in our parser
subbu has joined #jruby
<headius> Yeah that sounds right. No body is ever really empty, so it should get nil if there's no other instructions
<enebo[m]> HAHAHA OMG. I changed the script to do _e = $! and forgot
<enebo[m]> I was like wtf...this is not seeing this as an exc_var
subbu has quit [Quit: Leaving]
<headius> enebo: always put "fixes" in the body of the commit and it will link it in the commit and PR
<headius> fix is nice and simple, ship it!
<enebo[m]> headius: It did link on the actual issue with just subject but I keep forgetting about putting in body of message
<enebo[m]> then I don't want to edit original commit
<enebo[m]> hence the windy road I take
<headius> yeah I never put the bug number in the top line because git also prefers that line be under 60 chars or something
<headius> so short description of change in top line and long desc + fixes in description
<enebo[m]> I find it annoying it does not create a link on the subjectl ine for #nnn
<enebo[m]> I just like to see in git log subjects what it was for
<headius> yeah that is just a GH limitation I guess, but it is annoying you have to have multiple lines to make it work
<headius> would be nice to have bug numbers in subject but doesn't seem to be the preferred pattern
<headius> I guess the idea is that one-liners should not be a thing
<headius> so might as well keep subject short and specific
<enebo[m]> but the bug number is still nice outside their website
<enebo[m]> anyways it is not that important I just wish they would add a link
<enebo[m]> It links the important way too
<headius> it will also format commit subjects longer than 60 chars weirdly so I try to always obey that one
<headius> it = GH
<enebo[m]> yeah
<enebo[m]> it does the ... and just continues into the comment
<headius> doesn't leave a lot of room for "fixes #####"
<enebo[m]> vim seems to warn me at 60
subbu has joined #jruby
<headius> that timeout thing is a conundrum
<headius> it's like the exception is not being raised properly
<enebo[m]> pushing a merge soon jruby-9.3 -> master
<headius> No problem
<headius> The timeout thing seems to be a thread polling issue, the range each logic does not pull during any of the loops
<headius> So it is kind of a contrived case because nothing happens within the each body to trigger a thread poll but I'm guessing this is copied verbatim from the spec
<headius> And the each loops should probably be checking after each iteration anyway
subbu has quit [Quit: Leaving]
<headius> unsure if we should sprinkle all these polls in or just fix the trivial case in the example
<headius> if the rule is that we should be checking for thread events on loop backedges, it's arguable that we should add all of them
<puritylake[m]> How do you run a single spec file again?
lopex[m] has quit [*.net *.split]
bastelfreak has quit [*.net *.split]
<headius> spec/mspec/bin/mspec ci path/to/spec.rb
lopex[m] has joined #jruby
<puritylake[m]> Thanks
<headius> ok calling it a day for now, bbl
bastelfreak has joined #jruby
<puritylake[m]> Bye
subbu has joined #jruby
<puritylake[m]> You mentioned something about kwargs for JRuby enebo right? I assumed this can't be resolved until then?
<puritylake[m]> Never mind, I think I've figured it out