adam12 has quit [Quit: The Lounge -]
adam12 has joined #jruby
razetime has joined #jruby
razetime has quit [Ping timeout: 265 seconds]
razetime has joined #jruby
sagax has quit [Ping timeout: 246 seconds]
<headius> Good morning
adam12 has left #jruby [The Lounge -]
subbu has joined #jruby
<headius> enebo: k11chi
<headius> er
<headius> kiichi's latest two PRs look like good fixes but add some allocation... in the key/value hash calculation for example, every element in the has will allocate a new 16-byte array
<headius> I'm not sure how concerned we should be but I predict we'll be trying to find allocation-free solutions in the future if we don't address it now
<headius> enebo: also, looking into failures from
<headius> seems like something changed in a line number somewhere that should not be related to my changes
<headius> running that locally does not fail for some reason
<headius> ok most of these thread to_s failures do not pass on Ruby 3.1 either so I am adjusting expectations to match
<headius> same for some include-related tests in JRuby suite
<enebo[m]> headius: yeah the alloc was part of the reason I did not just merge it
<enebo[m]> a thread-local maybe is as good as we get for that particular code snippet
<enebo[m]> I am <80 F/E on MRI ripper tests but am mildly getting pulled down a rabbit hole
<enebo[m]> One I ducked down before though. I think I will figure it out quicker this time
<enebo[m]> In any case parser is working and ripper is mostly working
<headius> enebo: sounds like progress at least
<headius> I'm going through these failures and finding a bunch of stuff that doesn't match MRI but didn't fail on MRI because we guarded it for JRuby
<headius> so basically we match MRI now and fail our own patched tests as a result
<enebo[m]> headius: massive
<enebo[m]> It is very intricate to debug. You have one file which two parallel and one shared piece of code which generates its own types for which version of the parser you make. Then when something is wrong and you have to look at MRI's parser is is the same thing but different
<headius> so this will align our one parser file to rule them all with CRuby yeah?
<enebo[m]> heh the parallel is a ripper snippet and a regular snippet which may or may not come after a common snippet which runs before those snippets
<enebo[m]> It does but the ripper part is 90% the exact same text as the MRI one
<enebo[m]> So for 3.2 I just update all the text except for that 10% and figure out what needs to be done for that
<headius> enebo: I don't see anything in these failures that would stem from my change other than the minor patch to an include-related test of ours, so I'm going to fix that and merge and then fix the thread to_s stuff in a separate PR
<enebo[m]> /*% ripper: case!($1, in!($4, Qnil, Qnil)) %*/
<enebo[m]> This is the ripper-specific code in a production. case! becomes p.dispatch("on_case", $1, p.dispatch("on_in", $4, null, null));
<enebo[m]> Note: Not really this actually generates more lines of code for this but that is the gist of how it works
<enebo[m]> but anything like this is a no-work update if it changes. I just add it and it generates as it should
<headius> aha kiichi fixed some of these thread things
<headius> yeah nice... anything to make maintaining parser and ripper easier will be a big help
<enebo[m]> If I was so motivated I could try and combine the lexers but this is reasonable for now
<headius> such surprising things that have always failed, like how we assign a filename to a new thread
<headius> it was using only interpreter state, so if the method creating the thread was JIT or AOT compiled it would get the wrong filename and line
<enebo[m]> heh
<headius> now it uses block.getBody().getFile() so it didn't even need to use caller() logic
<enebo[m]> the whole wip track ended up causing a lot of old issues to be fixed
<enebo[m]> which may also tell you how important some of those failures were but it is nice to see how this caused us to "circle back"
<enebo[m]> oh that is nice!
<headius> kiichi actually adjusted our behavior to our tests so there were a few things still wrong
<headius> I have a commit fixing the test to pass on CRuby and now we will match
<enebo[m]> nice
<enebo[m]> yeah so far nearly all fails on our tests are literally us having bad tests bcause behavior has changed
<enebo[m]> That would be an simple enough beginner task would be to try and make any jruby tests which are not purely implementation to be added to spec and deleted from our suite
<enebo[m]> not high value but the shorter our test runs go the better
razetime has quit [Remote host closed the connection]
<headius> for sure
<headius> I almost just deleted these since there are bound to be other tests for Thread#to_s but I did not want to put the work in to make sure
<headius> a large percentage of our tests are actually very old copies of CRuby tests that have been mutilated by us over the years
<enebo[m]> yeah the unit tests
<enebo[m]> but even a bunch of our specs are not entirely us covering our impl
<headius> jruby suite should return to green on that one
<headius> enebo: so next week I'm in Morocco talking about JVM features through a JRuby lens (loom and panama mostly but I'll also cover some historic features like invokedynamic and scripting)
<headius> I will be online and working when conf stuff doesn't keep me busy
<headius> 50 min talk so I have to song and dance more than usual
<enebo[m]> ok
<enebo[m]> Presumably I will be done with this parser work and then will do expression evaluation order
<headius> yeah that is one of the few major items left
<headius> that is a little tricky to solve... running from within a complete jar it seems to use full path, probably pulling from classpath
<headius> perhaps we just make the test a little more forgiving of leading path elements?
<headius> hmm actually not a complete jar issue... also failing in the jit/aot mode tests
<headius> oops and interpreter... so this is just an issue of us getting the full path where CRuby uses the partial path
<headius> worth fixing? I am dubious
<enebo[m]> yeah those make me wonder...absolute seems better
<headius> yeah I can go both ways... absolute may expose file paths you don't want to expose
<headius> I might be able to fix it by getting filename from the scope instead of the body
<enebo[m]> Is there any scenario where relative would be what you want? Maybe but it is far-fetched like you want relative error to open a file in another directory with the same file in it
<headius> scope is created in parse and used by interpreter for this
<headius> body filename seems to end up with absolute path
<headius> well my thought is that if you have an error that shows absolute paths to files you are exposing information about your local file layout
<enebo[m]> I guess tooling which would not be capable of using an absolute path?]
<enebo[m]> ah yeah. So possible security issue if someone can read your logs
<headius> absolute for tooling would be appropriate but perhaps not for error reporting that might bubble out
<enebo[m]> which also feels like a stretch
<headius> yeah
<headius> I will try this though
<enebo[m]> but that is more common than my made up thing
<headius> ugh
<headius> ok so this only fails when run within our rake task
<headius> it may be a side effect of how it invokes the running, since it's probably giving full paths in the full suite but relative path when run at command line
<headius> I am going to modify this to just use FILENAME to match
<headius> er, the dunder version of that
<headius> (scope filename is the same so did not fix it)
<headius> ok I think I have it now
<headius> FWIW CRuby fails the test the same way if you give it the absolute path to the test when running... so the test was still broken
<headius> looking better now
<headius> ok looks like we're back to peak passing... remaining fails are a ton of stuff in MRI tests (we need to start evaluating what needs to be fixed), our slowly shrinking WIP specs, and the sequel and concurrent-ruby fails
<headius> 300-something in MRI but I see a bunch of things that are probably MRI specific
<enebo[m]> oh the didyoumean tests for pattern matching is something involving the didyoumean module gettign swapped out
<headius> in MRI suite?
<enebo[m]> If you run those tests by itself it all works so it is just something with parallel execution
<enebo[m]> yeah
<enebo[m]> I know they are not real errors and when I looked briefly it was obvious something was killing didyoumean
<headius> enebo: aha ok
<byteit101[m]> I need to buckle down and make a clean, gemified remote/Multi IRB thing. I've made dozens of project specific ones over as many years. JRuby's Readline support has always held me back before, but I hear that's improved in 9.4 so I should try again
<headius> Yeah the dodge in 9.4 is that we support the new mostly pure Ruby reline library
<headius> It calls out to a minimum of native console features through io/console which we implement with FFI
<headius> Basically all the console-tweaking stty features
subbu has quit [Quit: Leaving]
subbu has joined #jruby
subbu has quit [Quit: Leaving]