<lopex[m]>
we dont have a clue if it's fixed width or not (wrt surrogate pairs)
<lopex[m]>
I know utf-16 is a rare case but..
<lopex[m]>
but we could mark mark it as fixed width 99% of the time
<lopex[m]>
headius: once mentioned we could propagate utf-16 quite deep into jruby paths, I wonder if it's doable
<lopex[m]>
on java side though we could skip a lot of transcoding..
sagax has joined #jruby
<headius>
Or at least the transcoding would just be a memory copy
<headius>
It would be nice to handle homogeneous UTF 16 with a constant stride. That could either be a new code range or we alter the 7-bit marker to simply mean constant stride at the low end of the giving encoding
<headius>
Low end as in minimum char width
sagax has quit [Quit: Konversation terminated!]
<lopex[m]>
yeah, for now we have two bits for code range, but we have spare space for that
<enebo[m]>
perhaps it assumes empty before | must have something in it but the return value is strange
<enebo[m]>
ok
<enebo[m]>
ok was not for this channel :)
<headius>
so the latest item discovered by the indy runs is that indy call sites were doubling up the symbol in method_missing calls, but only when calling the ones that error
<headius>
the core ones
<headius>
we never noticed it because it only shows up in the exception thrown, which brings along the original arguments
<headius>
we will want to look into m_m again soon because it's super inefficient along most paths right now
<headius>
non-indy CachingCallSite constructs a new MethodMissingMethod wrapper whenever the m_m is user-defined... for every call
<headius>
and it caches nothing
<headius>
indy tries to do a better job but it's still calling the method along a varargs path and I need to profile it more
<headius>
enebo: do you still have a macos machine around?
<enebo[m]>
headius: in theory
<enebo[m]>
I can see if it boots but it has not been on for a couple of years
<headius>
I just filed a bug for this hanging UNIXServer#accept_nonblock but not sure if it is specific to macos on arm64
<enebo[m]>
wow this git pull is taking a while (new tag 9.2.20.1)
<enebo[m]>
hahahaha I forgot how loud this mac got
<enebo[m]>
headius: OMG that was painful
<headius>
yeah I've been loving the MBA with no fan
<enebo[m]>
runs to completion with two pends
<enebo[m]>
for whatever reason latest maven on that ancient mbp hates pom xml descriptor files made from windows
<headius>
ok so it seems to be macos/arm then that is hanging
<headius>
actually you just pulled master
<headius>
the two pends may be the things I masked
<headius>
remove the pend from those and try again
<enebo[m]>
It marks them as ISO8859_1 but it contains \r\n so it barfs mention UTF-8 BOM
<enebo[m]>
which is not a BOM but somethign weird
<enebo[m]>
so I correct them to UTF-8 and they load fine
<enebo[m]>
That machine still works on a very old version of MacOS and it feels slow...super super slow
<enebo[m]>
I am guessing last supported version of MacOS was just too much for it
<enebo[m]>
oh ok I will rerun
<enebo[m]>
both those tests pass
<enebo[m]>
headius: ^
<headius>
ok
<headius>
yeah it is weird how slow old macs feel now and I don't think it's just my perception
<enebo[m]>
they did not upgrade their chipset/cpu for years so it probably felt like more and more of a rip off by the time that update did happen
<headius>
enebo: I think I'm going to merge in this round of indy optimizations since it is green and improves a number of things
<enebo[m]>
cool any numbers for lopex?
<headius>
for most of it the benefit was smaller bytecode but I'll see if there's something I could benchmark
<headius>
big thing was moving call info update into the indy call site
<headius>
I was going to push a test commit to see if I could omit setCallInfo sometimes but I'd rather just merge what I have working and experiment separately
<headius>
I also improved how we do method_missing for non-indy CachingCallSite so it is zero-alloc for simple arities and calls
<enebo[m]>
cool
<headius>
hmm found a new area of exploration: ReceiveKeywordsInstr
<headius>
enebo: so in a case of `def foo(a) ... end` it appears to end up calling receiveSpecificArityKeywords
<headius>
looks like it does it for any fixed arity larger than 0 regardless of keywords being used
<headius>
is that what you would expect?
<headius>
the jit logic is different from interpreter so I'm trying to reconcile that
<headius>
different in that the interpreter has one big ass method
<enebo[m]>
that code is complicated but I think there are cases where if you get an actual keyword arity still needs to error
<enebo[m]>
so you have to at least make sure you do not have a specific arity but also are sending it a kwarg
<headius>
ah right
<enebo[m]>
I think the other case is it MAYBE can be marked as ruby2keywords method and then it needs to make the last hash a kwarg
<headius>
it's a lot of code for every fixed arity method entry
<headius>
I have not dug into the jit side of this stuff much since we shipped 9.4.2 so mostly just getting a feel for it
<enebo[m]>
oh heh...yeah so since this method may dupe it ends up potentially replacing last arg with a new object
<headius>
yeah makes sense
<enebo[m]>
but yeah I get it...this is a lot of bytecode
<enebo[m]>
I think we could solve this by specific arity method and specific arity if it happens to be ruby2keywords method
<enebo[m]>
I would have to check but part of the error handling is if you pass a kwarg to this method if will replace it with %undefined
<headius>
yeah I see the undefined
<enebo[m]>
check arity will recognize this and throw proper 2 for 1 sort of error
<enebo[m]>
Admittedly I found this solution workable but not really desirable. Making these as instrs to process kwargs is the most straightforward way without completely changing how IR works
<enebo[m]>
We talked years ago about having parameter binding preludes
<headius>
yeah
<enebo[m]>
more or less that we can still do analysis with instrs but the actual code for the binding is something other than hitting those instrs so literally
<enebo[m]>
I have a parallel but not quite lined up idea on this
<enebo[m]>
I thought about CISCing IR via macros
<enebo[m]>
so if I see a large pattern in AST I make a macro which represents it
<enebo[m]>
So interp gets quicker and smaller memorywise
<headius>
I'm contemplating what this should look like
<enebo[m]>
but it has to be able to "explode" the macro into a set of instrs for compiler passes
<headius>
we do this arity checking in IR for intepreter because everything comes through as arg[]
<headius>
but for specific-arity calls in indy the arity check is inserted around the call so it can be direct when they match
<headius>
I think that is the key here... being able to have the call site determine whether the keyword check is necessary since it can see both caller and callee
<enebo[m]>
the place where is breaks down for this with parameter binding is that we would explode the macro to JIT it so it would need to know nothing had changed so we could just have macro emits
<enebo[m]>
yeah and that must also include recognizing a method became ruby2keywords
<headius>
like right now I emit a vararg wrapper and the specific-arity method as two entry points
<headius>
the vararg wrapper just does the arity check
<enebo[m]>
but for specific arity routing keywords to a different emitted method could just arity error in that version
<enebo[m]>
but it would require minimal check somewhere to dispatch
<headius>
if you are calling with 3 args and the target has 3 args you do not perform any check
<enebo[m]>
well that is what I would like to see but we just have to know all scenarios where it needs to be more involved
<enebo[m]>
but I support the idea of having proper pure n arg methods
<headius>
right
<enebo[m]>
but we need some smarts to make it go more ordinary route
<enebo[m]>
I mostly just made this fit once I started seeing errors after things JIT
<headius>
right
<headius>
if it is a single-arity method and you pass a kwarg it has to error
<headius>
so that check has to happen somewhere
<enebo[m]>
but the logic of kwargs is dual in the sense there is pure ruby 3 syntax and there is dynamic ruby2 compat garbage
<headius>
we can optimize it if it can see the caller and the callee, so that means it has to be in the call site
<headius>
when we see no kwarg on caller and fixed arity on receiver then we do nothing
<enebo[m]>
so I think scope iskwargs (like we talked about last week) as a quick check to dispatch would allow pure specific atrity which omits that call and all the logic
<enebo[m]>
one half is statically determined but the other we cannot know upfront. any method of either should not hit specific arity
<headius>
yeah I'm thinking I emit a new signature that receives callInfo
<enebo[m]>
the for arguments themselves it is just checking that TC CallInfo constants as a quick check for specific and perhaps you can eliminate that in cases?
<headius>
if the method doesn't take kwargs then it just errors for any non-zero callInfo
<enebo[m]>
yeah that is what I want and it is semi what MRI does (although they do not pass the info but merely that it is a keywords call)
<headius>
if I can see at runtime that I have no kwargs on caller side and a target method with a no-kwargs path, I call that
<enebo[m]>
I want callInfo because then we can make native methods which just call that version
<headius>
otherwise we start calling the kwargs path with callInfo from indy call sites
<enebo[m]>
for the new -> initialize
<headius>
it is a partial way to pass through kwargs
<enebo[m]>
but yeah it seems reasonable to me but it will be fun to see if anything trips things up
<headius>
it is passing the metadata through but still with "packed" kwargs in the arg list
<headius>
later on we can "unpack
<enebo[m]>
and if you want to examine the logic in those recieve*keywords helpers you may realize I could have consolidated logic
<enebo[m]>
I feel like it could be simpler but it is not simple :)
<headius>
yeah I read through the big ass method
<enebo[m]>
callsite on both sides though is transformative to optimizing this
<enebo[m]>
err seeing both sides
<headius>
so I notice in the case of def foo(a) it gets that undefined back but never uses it
<headius>
there's probably a simplification there if method is fixed-arity with no kwargs
<enebo[m]>
checkAritySpecificArgs but how does this work
<enebo[m]>
I guess I do not recall how this ends up an arity error but fwiw specific arity check does ignore them
<enebo[m]>
ok no indefined in this version
<enebo[m]>
logic seems to be if kwargs is passed into the methodi it will dup it. And if the methods is ruby2keywords it will mark that dup
<headius>
right I see the logic that does need to stay here
<enebo[m]>
I believe this logic could be smaller just examining it
<headius>
if it has a last hash arg and it is kwarg then it needs to dup them
<enebo[m]>
it should be !(Hash) || callInfo_not_keywrds) return last;
<headius>
kwarg caller => trailing hash argument if arity matches
<enebo[m]>
and actually || keyword rest
<enebo[m]>
which could combine both of those || into a single bit checkl
<enebo[m]>
then it would be a single dupFast and an extra check to see if it should mark ruby2keyword on the new hash
<enebo[m]>
none of this helps you per se but it will look a lot simpler
<enebo[m]>
but if you have keyword check in front of all this then this will not need to be called at all
<headius>
yeah I am starting to see how the ends can get wired together
<headius>
trying to figure out if there's a way to do this a bit at a time
<headius>
like only for specific-arity add a call path only in indy
<enebo[m]>
I think slow path anything in here which causes a dup to happen would be a good step
<enebo[m]>
and not a big step
<headius>
right
<enebo[m]>
since it would just remove this method altogether at the cost of checking keyword flags and scope as to whether it is a kwargs method
<enebo[m]>
but either of those just is normal method path
<enebo[m]>
then this method goes away completely
<headius>
so if I have a callInfo = 0 in hand and a no-kwarg target I should be able to make the call without doing any of this
<enebo[m]>
I believe so. I don't know what happens in case of **{} but right now we omit that by bifurcating the call
<enebo[m]>
I do want to eliminate that
<headius>
that would not show up as specific arity I assume?
<enebo[m]>
Any non-kwargs method ignores it like it was not sent it
<enebo[m]>
mri31 -e 'def foo(a); p a; end; foo(**{})'
<enebo[m]>
-e:1:in `foo': wrong number of arguments (given 0, expected 1) (ArgumentError)
<enebo[m]>
I am using a literal there but it is dynamic most of the time
<headius>
ah sure, but that would have a flag for kwarg rest on the call side
<enebo[m]>
it wuold be I think KEYWORDS and KEYWORDS_EMPTY
<enebo[m]>
I am not sure if rest would be set or not but I guess EMPTY is all that needs to be there
<headius>
ok
<enebo[m]>
problem is that it is **hash normally
<headius>
I see it
<headius>
CALL_KEYWORD_REST
<enebo[m]>
So it will be kwargs and restkwargs but may get ammended to be empty
<enebo[m]>
because we don't know unless we look to see if it is empty or not
<enebo[m]>
although empty COULD maybe replace state with only empty
<enebo[m]>
mri31 -e 'def foo(a); p a; end; foo({})'
<headius>
ick
<headius>
**{} actually emits a hash
<enebo[m]>
lol anyways we will not pass the empty **{}
<headius>
in JIT
<enebo[m]>
but **{} is insanely rare other than test suites
<enebo[m]>
no one does it
<headius>
I suppose it has to because it is {} passing as args and ** as a flag in callInfo
<headius>
yeah
<headius>
I won't sweat it but I see how it works
<enebo[m]>
**hash and in this case it could make an empty one but as it stands now we make an if/else in IR which does not do it
<enebo[m]>
(which as I keep saying is undesirable)
<enebo[m]>
but we never create a hash we will throw away
<enebo[m]>
and the state is intended to eventually be possible to not make it but still pretend it is passed
<enebo[m]>
heh also consider foo(a: 1, **{})
<enebo[m]>
so all three states will be set but it really has more than EMPTY
<headius>
yeah ok
<enebo[m]>
if you consider your example earlier that will make {a: 1} and not care about the **{}
<headius>
one ugly way to do this would be to make all callInfo != 0 call through a varargs path
<enebo[m]>
I am not sure if I ever reasoned out why they allow empty kwrest to any method
<headius>
and then only put kwarg receiver logic in varargs
<enebo[m]>
yeah I think that is a reasonable first step
<headius>
they're already allocating a stack of objects for hash anyway
<headius>
that will allow existing call that are callInfo == 0 and specific arity to just go back to doing nothing
<enebo[m]>
dead simple arity methods will have no cost and then it just makes it a question is there somethign a little extra which can be done
<enebo[m]>
truth is kwargs passing will be cmoplicated so if there are other forms which can avoid the complexity of compiling that then that check might grow
<headius>
short term, we can overload varargs call path with unpacked kwargs too if we know to receive it right
<enebo[m]>
but at worst it will just be a more complicated bit compare on the flag side
<headius>
well medium term
<headius>
so we'd always varargs on kwargs paths but no hash
<enebo[m]>
It may make sense to have a small switch between simple, simple kwargs 1:1 binding, everything else
<enebo[m]>
combining simple with simple kwargs will always add some size/time to a whole class of simple methods
<headius>
yeah reducing this from allocating a Hash to allocating IRubyObject[] would already be a big win
<enebo[m]>
straight 1:1 kwarg binding is really a case of knowing and removign the hash
<headius>
and it will escape analyze better
<enebo[m]>
but it will be a different signature
<headius>
there's a lot of endpoints to coordinate
<headius>
I won't attempt that yet
<enebo[m]>
If you know if it is a:,b:,c: on both sides you don't even need the array
<headius>
for this just moving kwargs call + receiver check into varargs path as an experiment
<headius>
yeah true
<headius>
straight through
<headius>
with some metadata to say which is which
<headius>
I am passing five kwargs and here's their positions in the args array
<enebo[m]>
After talking this afternoon I think you can do one single callInfo check and remove this complicated method
<enebo[m]>
(well and also if the method tiself uses kwargs
<enebo[m]>
I guess too you need to reset the value if it does specificarity
<headius>
yeah
<headius>
it is too much to remove in here without having both caller and callee in hand
<headius>
so it must get lifted up call stack a bit
<enebo[m]>
That though as we said last week could possibly switch to start of callsite always setting vs resetting on way to call
<enebo[m]>
but for that to work we need something like callMethod(...., callInfo)
<headius>
yeah
<headius>
which I can simulate with indy but it needs larger changes for non-indy
<headius>
the JIT already emits specific arity path as a special method handle, so as long as the varargs path has kwarg checking I can drop kwarg checking in the specific arity version
<enebo[m]>
I think one issue is implicit calls for coercion
<enebo[m]>
if I call method foo and it decides that what I passed should be a string it may then call to_s. If foo is really .new then I have to worry about nuking callInfo
<enebo[m]>
I think callInfo is being too clever out of the desire to not change a million method signatures but doing it this way is brittle
<headius>
I see what you mean about ruby2_keywords now... it needs to be static or force a recompile
<headius>
because we compile earlier than it is set, it is passed in as state from interpreter
<enebo[m]>
yeah I was trying to say that last week
<enebo[m]>
if it force recompiles on set then that is good because it allows removing the check
<headius>
right
<enebo[m]>
I do not think we need to worry about races but JIT is off thread
<headius>
I mean recompile the IR too
<headius>
my last method_missing rework on PR has a few flaws so I'll get that green before merging and looking at new stuff
<headius>
oops
<headius>
no I actually committed the experiment to omit call info = 0 when target scope is not kwargs and is not ruby2_kwargs
<headius>
spoiler alert: it doesn't work
<headius>
but it seemed to fail for a method that definitely does receive kwargs and with callinfo != 0 so something's weird about that... it's the wrong experiment anyway
<headius>
ok I'm going to call it a night here and let this test run through
<headius>
tomorrow or tonight I will merge and try an experiment with call info and specific arity