github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
02:00 timotimo left 02:07 timotimo joined 06:00 MasterDuke left 06:25 robertle joined 06:55 committable6 left, bisectable6 left, committable6 joined 06:56 bisectable6 joined, evalable6 joined 07:13 lizmat joined 07:19 lizmat left 07:23 lizmat joined 07:35 lizmat left 07:58 lizmat joined
lizmat .ask jnthn does something Hash-ish need to mutable or not 08:17
yoleaux lizmat: I'll pass your message to jnthn.
samcv good * 08:32
jnthn: removing the previous pointer gives me almost a 1% speedup on my hash performance test 09:46
not giant but it does have some impact
dogbert11 samcv: are you still awake? 10:37
samcv yep
i'm in NL right now so it is about noon here
dogbert11 ah, cool
got a MoarVM Panic in t/spec/S15-nfg/concat-stable.t - Collectable in fromspace accessed 10:38
could possibly have something to do with arrays 10:39
#1 0x00007ffff7680e6c in push (tc=0x604a20, st=<optimized out>, root=0x7ffff2d17598, data=0x7ffff2d175b0, value=..., kind=<optimized out>) at src/6model/reprs/VMArray.c:497
could it have something to do with your recent opts? 10:40
samcv is it reproducable? 10:41
also it could
dogbert11 not 100 percent, you might need to run it a few times with MVM_GC_DEBUG=1 10:43
here's a gist - gist.github.com/dogbert17/796acab7...dff8a2ce93
samcv dogbert11: that's with the env var MVM_GC_DEBUG or the ifdef? 10:45
dogbert11 ifdef in src/gc/debug.h
samcv ah ok
i would think that might fix it if it were caused by me
dogbert11 you might be totally innocent :) 10:46
samcv clang gives me warnings with MVM_GC_DEBUG 10:50
about (*item_to_add)->flags & MVM_CF_STABLE == 0 && !STABLE(*(item_to_add))
because the == is evaluated before the &
though if i 'fix' it and put parents around item_to_add->flags & MVM_CF_STABLE then it triggers 10:51
dogbert11: 10:53
else if (((*item_to_add)->flags & MVM_CF_STABLE) == 0 && !STABLE(*item_to_add)) \
MVM_panic(1, "NULL STable in item added to GC worklist");
is ^ what was intended?
dogbert11 good question, I guess jnthn should answer that 10:56
10:57 mst left, mst joined, ChanServ sets mode: +o mst
samcv i mean it wouldn't make any sense otherwise, but i'm not sure how significant of a bad thing this is 10:59
adding a NULL STable
timotimo i think it's more that an object without an STABLE is added
i wonder if heaped frames are like that 11:00
then we'd just need an extra check there to see if it's a frame 11:01
well, gc_allocate_frame doesn't set an STable on it 11:02
samcv: ^- i think the right fix is to add the parens, but also check that MVM_CF_FRAME is not set 11:03
samcv timotimo: well is that checked on the line above? 11:04
if (*item_to_add)->flags & MVM_CF_FRAME && !((MVMFrame *)(*item_to_add))->static_info
else (the thing we've been talking about)
or do you mean only checking that the flags are not set as MVM_CF_FRAME?
ah i get it, yeah since that's an exception
timotimo right 11:07
samcv gotcha 11:08
dogbert11 cool, we should definitely run som spec/stresstests after this fix 11:11
*some
who knows what might turn up
samcv :)
Geth MoarVM: 3e91dc622a | (Samantha McVey)++ | src/gc/worklist.h
Fix MVM_GC_DEBUG worklist check for NULL STable

Previously there was an issue since == has higher precedence than `&` and this would never be triggered. Fix this by adding parens and making it do what it was meant to do originally as suggested by timotimo++
11:16
samcv dogbert11: ok this should fix that bug
timotimo the precedence of & or | and == has bitten me multiple times in the past
dogbert11 cool, will rerun
doesn't gcc warn about this? 11:17
samcv not sure. clang at least does. never seen it on clang 11:18
err never seen it on gcc that is
samcv almost always uses clang for MoarVM
mostly because the result is generally faster and the error messages are better. Though at times when i have been unable to figure out what's causing a specific compiler error such as when working on uthash which things go through about 5+ macros. i've switched to gcc and occasionally it would then have an error that made proper sense 11:20
a missing piece of syntax when using that many macros is really annoying to locate :)
or a missing backslash at the end of a line hah 11:21
dogbert11 I wonder if we have more '& or | and ==' errors int the codebase
samcv not that clang detects at least
dogbert11 cool 11:24
hmm, there are a few failures ... 11:26
let's see if I can repro them 11:27
Geth MoarVM: samcv++ created pull request #904:
Remove previous hash handle pointers in hash implementation
11:28
dogbert11 there's is still a problem 11:39
wrt gc that is
samcv dogbert11: can you try going back to before my gc changes?
dogbert11 I guess I can. Anyway, the stack trace is the same but the file is different in this case, i.e. t/spec/S32-io/indir.t 11:41
hmm, should try to find a test which fails often first otherwise this will be difficult I think 11:50
11:53 MasterDuke joined
dogbert11 samcv: I could definitely be wrong here but I believe that the problem was introduced with github.com/MoarVM/MoarVM/commit/0b...cd6d73c5d8 12:47
dogbert11 walk &
Geth MoarVM: d4d8ff4159 | (Jonathan Worthington)++ | src/6model/serialization.h
Mark functions used in Perl 6 extops MVM_PUBLIC

It started using these recently, and they need marking MVM_PUBLIC so they are available to the extops on Windows.
12:59
14:13 MasterDuke left 14:16 MasterDuke joined 16:19 domidumont joined 16:25 domidumont left 16:26 domidumont joined 16:39 zakharyas joined
lizmat FWIW, it feels to me that the latest bump in MoarVM makes things significantly slower 17:04
test-t race on my machine: .914 -> .947 17:05
that's about 3% slower :-(
17:05 zakharyas left 17:07 Ven`` joined
lizmat maybe it's just noise 17:08
17:16 AlexDani` joined 17:19 Ven`` left 17:20 AlexDaniel left 18:17 zakharyas joined 18:23 Zoffix joined
Zoffix I thought it was a bit slower than usual too when I wad bumping, but I thought it was the vm being slowet than udual. Aso stage parse for bump was 76s, but when I tried pre-bump version it was 71s 18:26
m: say 71/76 18:27
camelia 0.934211
18:52 domidumont left 19:04 zakharyas left 19:05 Zoffix left, zakharyas joined 19:08 zakharyas1 joined 19:09 zakharyas left 19:26 zakharyas1 left 20:11 MasterDuke_ joined 20:12 MasterDuke left 20:13 MasterDuke_ is now known as MasterDuke 20:36 AlexDani` is now known as AlexDaniel 20:50 MasterDuke left 20:52 MasterDuke joined 20:58 AlexDaniel left, AlexDaniel joined 21:29 lizmat left 21:32 lizmat joined 22:01 robertle left 22:26 stmuk_ joined 22:28 stmuk left
jnthn Probably any slowdown observed is due to the slightly more expensive deopt point algorithm that went in yesterday. Aside from costing a little more, it currently does an extra pass through the tree at fact analysis time, plus another pass during optimization time. 23:32
timotimo it's impressive that speshing things just a little later can make such a noticable difference 23:33
maybe we should do a little performance pass over spesh in the medium future
jnthn It's almost certainly possible to fold the first of those back into the existing facts pass at some point. 23:34
But when there's a pile of tickets about spesh-related regressions to look in to, I'll defer that.
timotimo maybe we should just really quickly put timing output into the speshlog for the individual phases of spesh apart from receive/analyze/plan/spesh/jit 23:35
sure
jnthn Also, while CORE.setting compilation maybe got a bit slower in *this* bump, I'd like to point out that I shaved that amount off it in earlier bumps. :P
And possibly more :) 23:36
Probably folding the extra pass back into the facts pass will win us most of it back,t hough
Also, better deopt analysis is a pre-req for being able to do box/unbox elimination across inlines. 23:37
So we're paying a bit more to be able to implement future optimizations 23:38
dogbert11 the offending commits are know for some of the reported problems, perhaps that makes finding the bugs a tad easier 23:40
*known
jnthn Still hopeful that I can get the analysis folded back into the facts pass, now I understand the algorithm.
dogbert11: Sometimes, yes, but when the commit is "mark invoke_v logged" then unfortunately the commit just exposes an existing bug that we could well have run into another way anyway, and just got lucky with. 23:41
Every time we make inlining more capable, we end up tripping over something that copes badly with that. 23:42
dogbert11 true, but at least it can be reproed ten times out of ten 23:43
samcv jnthn: if you can check out my hash PR github.com/MoarVM/MoarVM/pull/904/files or someone else may be ok but i think you know it better than others
though the changes aren't too complicated
jnthn Inlining is one of the most important things that we can do so far as optimizing Perl 6 goes. 23:44
dogbert11 I also believe that most stresstest related problems have something to do with github.com/MoarVM/MoarVM/commit/0b...cd6d73c5d8
jnthn Hmmm 23:45
That's interesting
In that it's only JITting something that we interpreted OK before
Wonder if I messed up the JITting...
dogbert11: If that's the csae, though, MVM_JIT_DISABLE=1 should makethe problems go away, in theory. 23:46
dogbert11 I don't think they do but lemme check real quick 23:47
jnthn samcv: I think by now you probably know it best. :-) Taking a look. 23:48
dogbert11 gdb output looks like this FWIW gist.github.com/dogbert17/796acab7...dff8a2ce93
jnthn ooh, that's interesting 23:50
samcv: Nothing in there stands out to be as being unreasonable 23:51
samcv: Though with all the macros the hashing code ain't the easiest thing to reason about... 23:52
dogbert11 no guarantee, but it seems as if the problem vanishes with MVM_JIT_DISABLE=1 23:53
jnthn dogbert11: What's curious is that the stack trace in gdb is from the interp, not JITted code 23:54
dogbert11 perhaps I'm blaming the wrong commit :( 23:55
jnthn I guess you were running with GC debug turned on?
dogbert11 yes, set to 1
jnthn Did you make an issue? I'm too tired to look at it now, but I'd be surprsied to get that output if there isn't a real bug. 23:56
dogbert11 samcv fixed i problem in that code (the gc debug code) earlier today
there is an issue by Zoffix about failing stresstests. R#2071 23:57
synopsebot R#2071 [open]: github.com/rakudo/rakudo/issues/2071 [⚠ blocker ⚠] Stresstest failures on v2018.06.179.g.9.dce.63318
dogbert11 I was going to add the above commit in a comment but perhaps I should refrain from doing that :)