00:11
AlexDaniel joined
01:49
ilbot3 joined
|
|||
samcv | ugh | 02:05 | |
this is what i have so far. need to figure out why i'm getting segfaults still github.com/MoarVM/MoarVM/compare/m...c?expand=1 | 02:06 | ||
MasterDuke | need moar MVMROOTS? | 02:09 | |
samcv | hm | 02:15 | |
ok well ifound one issue | 02:18 | ||
hmm yeah it seems to die when i try to get MVM_string_graphs on the returned string | 02:53 | ||
added an MVM root to the whole area but idk seemed to do nothing | 02:55 | ||
yep as soon as i try that it dies | 02:57 | ||
MasterDuke, so i set out to the string here github.com/MoarVM/MoarVM/compare/m...130f8cR462 | 03:02 | ||
and then i return. and i added prints and the same pointer is in both places but it causes a fault as soon as i access the graphs | |||
and i wrapped the whole area in MVMROOT(tc, renormalize_rtrn, { hmm | 03:03 | ||
maybe because it's not set to anything when i call MVMROOT... yeah that could do it maybe | |||
it only gets set to the right value when the function is called | |||
err god i'm dumb | 03:05 | ||
i gave the integer to MVMROOT not the pointer | |||
samcv facepalms hard | |||
MasterDuke | heh | 03:06 | |
samcv | well no segfault now. now it just hangs. so that's in improvement | 03:08 | |
and all my printf's get triggered | |||
it returns and then who knows what occurs | 03:11 | ||
oh it seems to loop in MVM_string_gi_get_grapheme somewhere someplace | 03:12 | ||
MasterDuke | what's the 'gi' mean? | ||
oh, grapheme iterator, right? | 03:13 | ||
well i'm outa here, good luck | 03:17 | ||
samcv | yep | ||
well it's somehow not composing it properly or something | 03:19 | ||
03:53
vendethiel joined
04:34
vendethiel- joined
04:44
vendethiel joined
|
|||
samcv | well ok i got it not GCing me all the time. made it a MVMString ** instead | 04:49 | |
now need to fix it trying to iterate past the end of the graphemeiterator | |||
now that i'm not getting corrupted information | 04:50 | ||
okay making some progress. not segfaulting and working in certain cases ;0 | 05:42 | ||
:) | |||
though semi limited | |||
06:11
brrt joined
|
|||
brrt | good * | 06:35 | |
samcv | good * | 06:40 | |
06:41
domidumont joined
|
|||
brrt is seeing your progress | 06:48 | ||
06:48
domidumont joined
|
|||
brrt doesn't know any better | 06:51 | ||
07:01
domidumont joined
|
|||
samcv | i'll get more to work on it tomorrow | 07:09 | |
issue right now is handling when due to the string concatenation, some of the strands turn out to be set to the same start and same end codepoint | 07:10 | ||
in which case it panics because it's not meant to handle that case so doesn't go to the next strand | 07:11 | ||
but i am pretty close i think | |||
it's at least working in certain cases so that's a whole lot better than failing horribly in 110% of cases :) | 07:13 | ||
07:13
brrt joined
07:18
domidumont joined
07:36
zakharyas joined
07:44
domidumont joined
07:46
lizmat joined
|
|||
brrt | so, meta-thing i learned yesterday | 08:08 | |
i've become somewhat of a heavy user of org-mode over the last few years | |||
for me it's an awesome way to manage and organize attention over an increasing set of jobs | 08:09 | ||
but yesterday i realized | |||
that's not an improvement per se | 08:10 | ||
it's an adaptation | |||
i used to be able to manage all these things in my head, and now i cannot | |||
samcv | brrt, what's org-mode? | 08:31 | |
brrt | why, it is awesome | 08:41 | |
it is an emacs 'major mode' that allows you to manage all sorts of stuff in a text file | 08:42 | ||
calendars, to do lists, papers, dependency chains, even has a spreadsheet | |||
exports to HTML, ODT, LaTeX | |||
orgmode.org/ | 08:43 | ||
jnthn | morning o/ | 08:53 | |
nwc10 | good *, * | 08:55 | |
jnthn | samcv: MAX_STRANDS is a soft limit so far as I recall; the idea is that if we end up with a huge number of strands then we'll collapse them | ||
08:58
brrt joined
|
|||
brrt | moarning jnthn, nwc10, samcv | 08:58 | |
jnthn is looking at the callsite thing and thinking, surely this can be done in a simpler/more robust way | 09:55 | ||
10:05
robertle joined,
brrt joined
|
|||
samcv | brrt, is it worth using emacs if i don't use it jsut for that? | 10:14 | |
jnthn | Grr, after trying some changes I now see more of why it's the way it is :S | 10:15 | |
brrt | well, that is useful, isn't it? | 10:21 | |
samcv, i can't say that for you. using emacs makes sense if you can get your brain to agree with it | 10:22 | ||
if you're a hardcore vim user, maybe not | |||
jnthn | Sorta, just means I can't get the simplificiation I expected. Now trying a different way. | ||
samcv | we i know how to use vim. but i mostly use atom | ||
not a hardcore anything | 10:23 | ||
brrt | right. if you use atom, then at least emacs isn't going to feel slow for you | ||
emacs has all the convenience you might expect, if you're willing to write bits of lisp :-P | 10:24 | ||
nwc10 | and you can run vim inside it./ | 10:36 | |
oh wait, that's not the point. :-) | |||
timotimo | for vim there's vim-journal or something. i've had it open in a tab for multiple months now, never actually tried it >_< | 10:37 | |
brrt | oh, you can have a vim-mode for emacs, yeah | 10:46 | |
i sometimes try to use vim for serious editing... | |||
just doens't agree with my brain as well as emacs does | 10:47 | ||
jnthn | Hm, it seems that the only way spesh may be involved in the bug in question is that due to its logging, it keeps some MVMCallsite objects alive for longer than they otherwise would be | 10:48 | |
And so they actually live long enough to be marked | |||
10:51
brrt left,
brrt joined
|
|||
dogbert17 | jnthn: are you looking at the destructuring issue? | 10:54 | |
jnthn | dogbert17: Yeah | 10:55 | |
dogbert17 | looks like you managed to do some code cleanup yesterday | 10:56 | |
dogbert17 wonders where all the 'one line patch bugs' has gone | 11:00 | ||
nwc10 | welcome to a mature project | 11:01 | |
jnthn | OK, interesting, it seems the extended lifetime is not, after all, an artefact of spesh logging, in that if I make sure we never log a call capture we still end up marking them | 11:02 | |
huh, turns out that I can golf the issue a good bit further | 11:11 | ||
I think when I did it before my criteria was "does it SEGV" | |||
But if it's "does it make valgrind unhappy" then a bunch of stuff goes away | 11:12 | ||
What notably does not go away is that it seems to need to be a multi-dispatch *and* to use destructuring | |||
My suspicion is starting to fall upon invokewithcapture | 11:13 | ||
And the effective callsite mechanism | |||
nwc10 | you've giving it a long hard stare? :-) | ||
brrt | n | ||
nwc10 | (don't forget the marmalade sandwich) | 11:14 | |
brrt | jnthn: have you checked if it's not JIT-specific | ||
jnthn | brrt: Yeah, it shows up just the same with MVM_JIT_DISABLE=1 | ||
But goes away with MVM_SPESH_DISABLE=1 | |||
brrt | bummer... | ||
jnthn | Which must be part of the puzzle too | 11:15 | |
nwc10 | what does your golf case look like currently? | ||
jnthn | I'm running it as MVM_SPESH_INLINE_DISABLE=1 MVM_SPESH_OSR_DISABLE=1 MVM_JIT_DISABLE=1 fwiw | 11:16 | |
gist.github.com/jnthn/5e7a5d8ebac1...d415b46efe | |||
Also it's possible to remove the @a and just flatten in the list | 11:18 | ||
And to remove the logging | |||
uh, the $i | |||
nwc10 | ASAN finds this code worthy of excitement: paste.scsys.co.uk/564605 | ||
s/this/that/ | |||
jnthn | Yeah, that's exactly the same finding valgrind has | ||
Like, precisely the same 3 stack traces | 11:19 | ||
nwc10 | OK. Good, in as much as we've quickly established that there wasn't anything missed. | ||
missed by valgrind but seen by ASAN | |||
so it's purely a heap error | |||
jnthn | Yeah | 11:21 | |
So the ingredients seem to be all of spesh, multi-dispatch, and destructuring | |||
Multi-dispatch and destructuring being notable in both using MVMCallCapture | 11:22 | ||
nwc10 | ponder it over lunch? | ||
jnthn | Yeah, think it's time to eat :) | ||
bbiab | 11:23 | ||
timotimo | when you have it open in rr, you can step forwards and backwards between where it gets created, where it gets freed, and where it gets overwritten later, and where the gc stumbles upon the bogus data in it | 11:27 | |
i didn't know what the next step in actual debugging should have been, though | |||
it might be interesting to have a debug mode where we allocate everything in gen2 (so that nothing moves) but we still gc every now and then (as if we were running normally) | 11:29 | ||
since this bug relies on having things gc marked in order to asplode at all | |||
gotta go | 11:30 | ||
jnthn back | 12:11 | ||
brrt | \o | 12:13 | |
jnthn | Turns out that commenting out MVM_spesh_log_add_logging hides the issue | 12:20 | |
Commenting out instead any of the differnet optimization phases spesh does has no effect on hiding the bug | |||
So it seems that it *is* that spesh logging elongates lifetimes, and so shows up something that'd be an issue anyway | 12:21 | ||
It just isn't that it's a call capture itself that is being logged, but presumably something that references it | |||
That also fits with the output in that we get an error from valgrind but then can continue without errors | 12:22 | ||
Yeah, my hunch that it might be about invokewithcallcapture seems to be it | 12:27 | ||
Got a working fix | 12:31 | ||
Downside being that it makes frames a pointer larger for a relatively uncommon case | 12:32 | ||
12:51
Catbert17 joined
|
|||
Geth_ | MoarVM: 7c5aaa3546 | (Jonathan Worthington)++ | src/6model/reprs/MVMCallCapture.c Remove out-dated comment. |
12:58 | |
MoarVM: 6aa01090e7 | (Jonathan Worthington)++ | 3 files invokewithcapture must keep the used capture alive The frame's argument processing context will come to reference the effective callsite. If it were to be collected before the frame was executed, then the frame would get a dangling callsite pointer. This was most noticable because a further MVMCallCapture could then come to refer to this same callsite, be marked, and the mark would cause a use after free. Making sure that it stays alive costs an extra pointer in MVMFrame, which is a bit of a pity, but less of one than SIGSEGV. |
|||
MoarVM: 3d59fac35e | (Jonathan Worthington)++ | src/6model/reprs/MVMCallCapture.c Remove more leftovers from capture use-mode. |
12:59 | ||
lizmat | jnthn: apart from using more memory, do you see any other downsides ? | 13:03 | |
jnthn | Not really, but I think this only kicks the can further down the road | 13:06 | |
In that there's still a way to get a dangling pointer | |||
lizmat | :-( | 13:07 | |
.oO( damn danglers ) |
|||
jnthn | Which is fixable by just making sure we *always* copy the callsite, never reference it | ||
Perhaps with an exception for those that are interned | 13:08 | ||
It's plausible that we can also use a copying approach to get rid of the extra pointer in frame too, thinking about it | |||
13:11
deep-book-gk_ joined
13:13
deep-book-gk_ left
|
|||
jnthn | Always copying the callsite seems to work out | 13:18 | |
(for fixing the remaining potential dangling pointer issue) | |||
It also makes MVMCallCapture 4 or maybe 8 bytes smaller (depends on alignment) | |||
*And* gets rid of a bunch of logic | 13:19 | ||
grr, some spectest issue | 13:21 | ||
jnthn does a pull on Rakudo just in case, 'cus it may have been a hang in a set test | 13:22 | ||
Though gotta go for langauge lesson now | |||
bbl | |||
brrt | (is the callsite a large thing?) | 13:26 | |
nwc10 | That (unconditionally) doesn't matter, does it? (for a delta value) The question is "are there lots of them?" | 13:28 | |
(I think) | |||
13:33
domidumont joined
13:39
ggoebel joined
14:35
AlexDaniel joined
14:45
brrt joined
|
|||
jnthn | Callsites in common use are interned. And no, they ain't so large. | 14:54 | |
Spectset was clean with HEAD Rakudo | |||
So guess I was missing a fix | |||
Hm, only question now is "why was that sepctest run so slow" | 14:55 | ||
timotimo | 90% time spent in copy_callsite? :o | 14:56 | |
Geth_ | MoarVM: 3dd74960ac | (Jonathan Worthington)++ | 4 files Always copy callsite into an MVMCallCapture. Rather than trying to reference it. That turns out to be reliably fragile, and it's not a big thing to copy. The reward is that we get to eliminate a bunch of logic, making for clearer code, and also lose a flag that - after alignment considerations - will save 4 or 8 bytes per call capture. |
14:58 | |
jnthn | Probably a debug build or some GC flag turned on :) | ||
timotimo | i tend to have optimize in gcc turned off, gives about 2x difference | ||
jnthn does a non-debug build and fires off another run just in case | 15:05 | ||
Zoffix | hm, I too had a 40%-slower-than normal spectest run :/ | 15:06 | |
timotimo | hmm, cosmic rays? | 15:07 | |
jnthn | huh, no, it was super slow again | 15:09 | |
15:09
brrt joined
|
|||
jnthn | Hm, startup maybe got a bunch slower? | 15:10 | |
Zoffix | parse was slower for me. 87s vs ~67s in the past | ||
jnthn | 40% is what I@m seeing too | ||
15:16
geekosaur joined
|
|||
Geth_ | MoarVM: 67202501f4 | (Jonathan Worthington)++ | src/core/args.c Clean up some duplicated code. The bind error handling can just call the code to save the arg capture instead of repeating it. |
15:17 | |
timotimo | i pulled latest moar and my time went up from 53 to 61 (stage parse) | 15:24 | |
brrt | correctness is costly... | 15:25 | |
Zoffix | brrt: is that what is is? | 15:26 | |
jnthn | I...can't imagine it being *that* costly o.O | ||
timotimo | did we accidentally throw out spesh logging? | 15:27 | |
jnthn | If we did, I can't see wehre | 15:28 | |
timotimo | right | ||
jnthn | I mean, can't see any evidence I comitted that by accident | ||
I'll take a look shortly | |||
timotimo | yeah, my theory was you perhaps acidentally left in a line or something | ||
didn't glance the diffs yet | 15:29 | ||
anyway, a perf report doesn't show something massive | |||
jnthn | Turns out I can shave another 4 or 8 bytes off MVMCallCapture too | 15:30 | |
Geth_ | MoarVM: 321eee7cb4 | (Jonathan Worthington)++ | 6 files Remove now-unrequired effective_callsite. It's always the same as the callsite pointer inside of the argument processing context, so don't duplicately store that. |
15:33 | |
jnthn | Anybody else working on hunting the slowdown? | 15:34 | |
timotimo | not really | 15:35 | |
jnthn tries 2f9be082 which is before his work today | 15:36 | ||
Ah, that's fast | 15:37 | ||
Anyone want the SEGV back? :P | 15:38 | ||
jnthn tries 7c5aaa3 | 15:39 | ||
Also fine | 15:41 | ||
dogbert17 doesn't notice any slowdown but that might be due to running on 32 bit | 15:43 | ||
jnthn | Testing 6aa01090 now | 15:44 | |
Minor slowdown perhaps due to larger MVMFrame or perhaps noise, certainly not the huge one we're hutning | 15:45 | ||
*hunting | |||
That only leaves 4 more commits | 15:46 | ||
Trying 3d59fac | |||
Which would be an odd one to cause it | 15:47 | ||
I suspect it may turn out to be 3dd7496 | |||
15:48
yoleaux joined
|
|||
dogbert17 notices that the number of tests being run have increased by several thousand in the last few days | 15:48 | ||
jnthn | Nope, not 3d59fac | ||
Seems it is that one | 15:50 | ||
Ohhh | 15:52 | ||
"oops" | |||
So it turns out if we always copy the callsites...we never end up with an interned callsite being seen on various code paths | 15:53 | ||
timotimo | oh, *always* | ||
i thought we never copy interned ones | |||
still after today's work i mean | |||
jnthn | No, I figured that was an optimization for when I'd got things working | ||
Turns out that I was right. :P | |||
brrt | hehe | ||
jnthn | Didn't realize it'd regress so much | 15:54 | |
timotimo | :D | ||
jnthn | But it turns out that it means we will never insert into the multi-cache | ||
timotimo | right, ouch :) | ||
multi dispatch can be expensive, yeah | |||
jnthn | Having done all the previous cleanups, it was easy to put the don't-copy-on-intern change it. | 16:00 | |
Yeah, that fixes the perf regression :) | 16:02 | ||
Geth_ | MoarVM: 2e683f95f7 | (Jonathan Worthington)++ | 3 files Don't copy interned callsites. Not only is it not required because their memory management is simple (they live so long as the VM does), but also a callsite that is not interned will not be entered into a multi-dispatch cache - meaning the earlier change to copy always introduced a huge performance regression that this commit addresses. |
16:06 | |
jnthn | And the thing I set out to fix in the morning is still fine :) | 16:07 | |
dogbert17 | jnthn++, that means that you've at the very least fixed MoarVM issues #612 and #608 | 16:10 | |
jnthn | Yup :) | ||
dogbert17 | and perhaps more considering you found a missing MVMroot yesterday | ||
jnthn | Yeah, maybe so | 16:14 | |
Closed those two | 16:16 | ||
Probably also github.com/MoarVM/MoarVM/issues/412 | 16:19 | ||
And github.com/MoarVM/MoarVM/issues/552 wow | 16:21 | ||
So, 4 issues down :) | 16:22 | ||
dogbert17 | and perhaps RT #128553 | ||
synopsebot6 | Link: rt.perl.org/rt3/Public/Bug/Display...?id=128553 | ||
jnthn | looks like, yes :) | 16:26 | |
Maybe the gist linked in there can be turned into a spectest :) | 16:27 | ||
Not by me right now though, I think it's time to rest and dinner :) | |||
I think I'll also work on putting MVMFrame on a bit of a diet later in the month also | 16:28 | ||
Given I added one more thing into it today :) | |||
afk for a while | 16:31 | ||
18:01
domidumont joined
19:14
zakharyas joined
19:38
agentzh joined
19:39
agentzh joined
19:47
AlexDaniel joined
20:10
robertle joined
20:25
agentzh joined
21:38
colomon_ joined
|
|||
samcv | it's alivvvveeee! | 22:02 | |
jnthn | o.O | ||
:) | |||
samcv | heh. it's working when concatenating things if it copies or doesn't copy | 22:04 | |
yesterday i finally got it working but it only worked when there were no copies | |||
of strands | |||
jnthn | :) | ||
Sounds good | 22:05 | ||
Rest time, 'night | 22:06 | ||
samcv | doesn't pass all tests though hm | 22:17 | |
gonna run spectest and hopefully will find something i can easily test and replicate | 22:19 | ||
\o/ yay it passes my concat test now :) | 22:37 | ||
exciting | |||
MasterDuke | and it's faster? | 22:38 | |
samcv | as long as both things are CCC 0 then we can do the shortcut and not re_nfg | ||
well it's gotta be faster | |||
it doesn't have to run re_nfg every single time renormalization is needed | |||
for now it only works when it joins into two codepoints, it won't trigger if the CCC isn't 0 for both codpoints. but it still catches most cases | 22:39 | ||
MasterDuke | CCC? | ||
samcv | i will time it once i make sure that spectests all pass | ||
canonical combining class | |||
though i might be able to only trigger it if the ccc on one side is higher than the other but this is pretty decent for now | 22:42 | ||
MasterDuke | so you created a fast(er) path for concatenating strings where the CCCs are the same and renormalization is required? | 22:43 | |
samcv | yep. it will combine the last codepoint of string a and the first codepoint of string b | 22:44 | |
and leave the rest alone | |||
so can save loads of renormalization depending on the length of the strings | |||
MasterDuke | but that is a slightly special case, right? i.e., `my $a = 'a' ~ 'b'` won't be any faster? | 22:47 | |
but will `my $a = 'a' ~ "\n"` be faster? | 22:48 | ||
samcv | hmm it seems to make something fail and i'm not sure why. but it occurs if it's \r + \n | 22:50 | |
and i just confirmed that it does generate the right synthetic grapheme | |||
but tests in t/spec/S17-supply/lines.t fail | |||
the test name is: "handle chunked lines" | |||
MasterDuke, looks like it will be probably faster for concetting two codepoints which concat into 1 as well | 23:01 | ||
like "\c[BOY]" ~ "\c[ZWJ]" for example | |||
if it consumes string a and string b then it returns early | |||
well since they stick together | |||
gonna update the whole toolchain and then try rerunning tests | 23:08 | ||
MasterDuke, but yeah simple cases aren't going to be faster. it's only renormalization gets triggered | 23:10 | ||
and i already made it trigger much less often, and now trying to make it so it doesn't have to renormalize *the entire of both strings* when you concat when it's triggered | |||
MasterDuke | cool | 23:40 | |
samcv | oh no MoarVM panic: Internal error: invalid thread ID 10787 in GC work pass | 23:46 | |
timotimo | hooray :\ | 23:47 | |
samcv | happening on a lot of roast files | 23:49 | |
not sure if my changes have anything to do with it | |||
ok i don't get it with master. odd | 23:50 | ||
why would my changes do that... | |||
it's always the same thread id. maybe i'm passing some wrong value as a thread id somewhere? | 23:51 | ||
ugh | 23:52 |