<enebo[m]>
fidothe: the \n replacement bit is still broken but I was curious on perf difference. It is a little bit faster but the extra calc into the offset does not really show up too much with ascii 7bit data (e.g. it becomes math)
<fidothe>
aha. i've been restricted on time for the last few days and i spent a bit of time looking at it today, but was stumbling over the byte-pos vs char-pos stuff. this is great
<enebo[m]>
err \0...\n not a newline
<fidothe>
enebo[m]: that's what i thought you meant
<enebo[m]>
I will try and update a bench which shows the cost of re-walking by putting mbc into a string.
<fidothe>
i am curious how we cope with indexing/looking up one string in a non-7bit-ascii encoding against one in a different non-7bit encoding
<fidothe>
i mean, generally... nothing has jumped out at me, which makes me think i don't understand the way the Encoding/jcoding/CodeRangeable stuff works well enough
<enebo[m]>
yeah that is a good question since pattern enc is what it uses
<fidothe>
(e.g. u-umlaut in ISO-8859-1 vs UTF-8)
<enebo[m]>
in general there is some sort of resolution between the encodings including potentially some transcoding but if you look at cat() impl you can see some pretty wild logic
<fidothe>
if you can normalize to the same encoding (i.e. convert pattern to same enc as string) then I guess you can do a straight byte-only comparison and skip the mbc stuf
<enebo[m]>
yeah that is a possibility but the cost of conversion itself could be a big cost
<enebo[m]>
but I don't know if it errors or works
<fidothe>
patterns would tend to be much smaller than the to-search string
<fidothe>
but that's probably one to look at once this works, and if perf is unnacceptable
<enebo[m]>
yeah
<lopex>
the case with unicode/ascii is that it's ascii compatible, so worst case scenario is comparing code points one by one
<lopex>
which isnt good though perf wise
<lopex>
er, I back that of
<lopex>
fidothe: yeah, 7bit + ascii compat should go the patch you described
<enebo[m]>
I added a mbc to show cost of having to walk for each substitute
<enebo[m]>
in single byte optimize that cost boils to math but this extra char shows lopex assertion about the cost of walking
<lopex>
enebo[m]: I think there should be a specialization for that anyways
<lopex>
what does mri do though ?
<enebo[m]>
Weirdly I ran branch and branch + path multiple times and the regexp case always did better in branch + path when naively you would assume it to be the same result as branch (since no code changed there)
<enebo[m]>
I guess maybe that reduced some confusion in the regexp code by not going through the codepath anymore in the first case
<lopex>
enebo[m]: those paths wont be optimized as they are in real apps
<enebo[m]>
yeah I agree with that
<enebo[m]>
lopex: I don't know what MRI does but I find it interesting we never did that optimization in the original index helper
<lopex>
enebo[m]: that code might be old, and there's many more places for such optz too
<enebo[m]>
lopex: although if this is all ascii then we just literally return pos + offset right?
<lopex>
enebo[m]: how does mri compare for that bench ?
<lopex>
enebo[m]: I dont recall
<enebo[m]>
ah I did not run since I added the mbc...I will try
<lopex>
enebo[m]: but the diff per wise is quite dramatic
<lopex>
enebo[m]: how long are the strings ?
<enebo[m]>
even long one is not very long...look at gist above
<lopex>
enebo[m]: mri usually shortcuts for cr7 bit in many impls
<enebo[m]>
The original benchmark used only single substituion on a tiny string
<lopex>
yeah, I see it
<lopex>
does string sub support hash replacements ?
<enebo[m]>
lopex: update gist for 2.7
<enebo[m]>
so we are way faster and this is without indy so we are not opting that while I added
<enebo[m]>
I take it back indy does not give us much so that while + method opts pretty good
subbu|lunch is now known as subbu
<enebo[m]>
The randomness of using graal ce is always a mystery....it is 30k i/s for gsub-string
<lopex>
so like 30% faster ?
<enebo[m]>
yeah
<lopex>
looking at mri code, and what I can see is all historical remnants
<enebo[m]>
If I had to correlate ce to c2 I would say ce tends to have much better results on smaller programs and tends to stay up or fall behind a little once it is full app size
<lopex>
like:
<lopex>
if (mode == ITER) {
<lopex>
if (mode) {
<lopex>
this is MADNESS
<enebo[m]>
IT ALL COMES OUT IN THE WASH
<lopex>
enebo[m]: probably graal takes more advantage of devirt and inlining for that one ?
<lopex>
or well, splitting
<lopex>
there's so many branches there
<lopex>
enebo[m]: you think is could constant propagate string type ?
<headius[m]>
CE was comfortably faster than C2 on that threadlocal bench also
<headius[m]>
I haven't looked at the asm to see what might be different
<lopex>
I think it's good at code splitting
<headius[m]>
without revealing any benchmark results, I'll just say EE was not as fast
<lopex>
for that gsub thning
<lopex>
though String#split could also be an indicator for graal perf
<enebo[m]>
lopex: I don't know
<lopex>
ok, no more speculations :P
<enebo[m]>
lopex: I think for 7bit path we would just eliminate what would inline to mostly simple math
<lopex>
enebo[m]: if specialized
<enebo[m]>
assuming it actually inlines
<enebo[m]>
well that's true
<enebo[m]>
lopex: after how long...more than a decade? I cannot tell you how common mbc unicode ends up in typical US apps
<enebo[m]>
Or EU apps
<enebo[m]>
I suppose euro unicode itself probably ends up causing some pain to make us go slow path
<lopex>
I guess graal could inline that without if only that path is taken in the benchmark
<lopex>
enebo[m]: it's insane here in the EU
<lopex>
enebo[m]: and the apps are still struggling with german 'ss'
<enebo[m]>
This particular version of index could pass in whether both strings are 7bit cr and just have an early exit to avoid rightAdjust++ logic
<enebo[m]>
err actually only source string would need to pass it in
<lopex>
enebo[m]: does this path go through areCompatible ?
<enebo[m]>
I cam calling thisfrom gsubCommon so I will have to trace back
<enebo[m]>
A future to gsubCommon could be to split it into several versions since it handles block/hash/string all in the same methods
<headius[m]>
those signature changes just spread like a cancer
<headius[m]>
hopefully latest push gets to a reasonable edge
<lopex>
tc ones ?
<headius[m]>
Yeah
<headius[m]>
I pushed a little harder to avoid having too many upstream callers do getCurrentContext but that's the sort of change that has to propagate
<lopex>
one would think it's easy to get them spotted
<headius[m]>
It's a delicate dance...we want to add the param but keep and deprecate the signature that doesn't have it...then need to find all callers and implementers of the old signature and fix them, and so on
<lopex>
hard dance
<headius[m]>
Impossible to totally avoid adding some getCurrentContext at least for the deprecated signatures
<lopex>
I remeber I used it to oblivion when joined the project
<lopex>
and had no clue what it means
<headius[m]>
it's probably faster now than it was then, especially with my recent tweak, but it will never be "free" like a parameter
<headius[m]>
and I'm sure it messes with CPU caches and register alloc in some way that a parameter obviously wouldn't
<lopex>
by polluting the cache you mean having all those things on ruby thread ?
<headius[m]>
well I mean inlining all the code for getCurrentContext's thread local access probably messes with register allocation and certainly loads structures that would otherwise not be needed
<headius[m]>
so it's going to push something out
<lopex>
yeah, certainly
<headius[m]>
where having context passed as a parameter will mostly be a no-op since it can just stay in a register for inlined calls
<lopex>
yeah
<lopex>
but when I joined the project getCurrentContext was just a name
<lopex>
funny
<headius[m]>
yeah we're still cleaning up decisions made before either enebo or I joined the project
<lopex>
well, I knew It had to do something with threadlocals
<lopex>
but who cared
<headius[m]>
🤷♂️
<headius[m]>
enebo: your unpack 'm' PR is green now
lucasb has quit [Quit: Connection closed for inactivity]
<enebo[m]>
headius: yeah I know. I will merge
<headius[m]>
doesn't seem like we've had any major regressions reported for this release...not bad
<headius[m]>
enebo: kares: btw I was thinking recently about how we change some behaviors when run from Main...I think some of that might be applicable to memory, JIT tweaks we've worked on recently
<headius[m]>
like if you run from command line, it's pretty unlikely you're going to be embedding a lot of other
<headius[m]>
...other JRuby instances, so we could potentially simplify classloading there
<headius[m]>
we already do this for a few things like hard exit from process