github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
00:05 pamplemousse joined
timotimo off to bed o/ 00:22
dogbert17 good night 00:23
00:46 lucasb left
discord6 <timotimo> As I'm lying here trying to fall asleep, it occurs to me that perhaps the nursing thing is a write barrier, I.e. that MVM_ASSIGN should be used to say the type in the guard entry 00:52
<timotimo> The root would be whatever MVMObject it belongs to, which I'm not sure which one that is. Perhaps the frame? But then if it's not on the heap, what happens then... 00:53
<timotimo> Or is return data an actual MVMObject? 00:54
<timotimo> Anyway back to sleep attempts
01:06 pamplemousse_ joined 01:09 pamplemousse left 01:13 pamplemousse_ left 01:15 pamplemousse_ joined 01:25 pamplemousse joined 01:28 pamplemousse_ left 01:50 pamplemousse left 01:55 pamplemousse joined 02:09 pamplemousse left
samcv ok. time for release 05:13
Geth MoarVM/master: 17 commits pushed by (Ben Davies)++, (Stefan Seifert)++, (Jonathan Worthington)++, (Timo Paulssen)++, Altai-man++, (Samantha McVey)++
review: github.com/MoarVM/MoarVM/compare/2...744a47a2a5
05:25
06:30 evalable6 left, unicodable6 left, greppable6 left 06:31 coverable6 joined, greppable6 joined, notable6 joined 06:32 shareable6 joined, bloatable6 joined, bisectable6 joined, statisfiable6 joined 06:33 undersightable6 joined, committable6 joined, nativecallable6 joined 06:34 benchable6 joined, evalable6 joined, releasable6 joined, unicodable6 joined, reportable6 joined 06:35 squashable6 joined, quotable6 joined 08:27 chloekek joined
nine A memset(other->nursery_fromspace, 0, other->nursery_fromspace_size); after GC yields some interesting results. Like a tc->cur_frame->caller->return_value pointing at a zeroed-out object tripping MVM_args_set_result_str 09:12
And suddenly I seem to find myself in the same area of code as dogbert17... 09:18
I'd be surprised if we didn't have to MVMROOT name and plugin here: github.com/MoarVM/MoarVM/blob/mast...ugin.c#L20 09:32
09:46 brrt joined
brrt \o 09:52
nine Oooh...the difference is: for me adding MVM_spesh_plugin_guard_list_mark(tc, srd->prev_plugin_guards, srd->prev_num_plugin_guards, worklist); to mark_plugin_sr_data actually fixes the issue!
nwc10 o/ 09:54
nine At least in some cases... 09:55
brrt hmmmm
probably multiple issues then
congratulations nine :-)
nine thanks :) 09:56
09:59 brrt left
timotimo nine: yeah, that does look like a place one would usually put an MVMROOT 10:00
probably difficult to hit as it'll only ever run once (per hll - so twice at most)
10:01 brrt joined 10:10 brrt left
nine Ouch...it's not just the target frame's return_value that's NULL. The whole target frame is zeroed out 10:28
And this happens in: method result_kind() { $!result_kind } 10:29
10:42 MasterDuke joined
MasterDuke anyone have an idea why `mp_lshd(<mp_int == 20>, 1)` would end up making the mp_int == 23058430092136939520? 10:46
dogbert17 nine: very interesting 10:52
timotimo: managed got get a stack trace from the 'last GC' before failure 10:53
s/got/to/ # need coffee
dogbert17 fixes the coffee bug by decisive action
timotimo: the stack trace from the last GC before the failure - gist.github.com/dogbert17/42f07a57...77651c0da2 10:57
11:01 sena_kun joined
nine God damn it C! Macros that look like function calls are devious. The segfault I traced was in autobox(tc, target, result, str_box_type, 1, set_str, target->return_value->o); which is not a function call but a macro. So the segfault didn't actually occur in this line but in the expanded macro. 11:11
And of course the macro gives an opportunity for GC and thus for target to move _before_ target->return_value->o is actually evaluated
dogbert17 nine++, you're finding bugs at breakneck speed 11:13
11:19 ggoebel_ joined
Geth MoarVM: 0a12e3c4e3 | (Stefan Seifert)++ | src/6model/reprs/ConcBlockingQueue.c
Fix ConcBlockingQueue possibly unshifting an outdated item pointer

An untimely GC may move to_add after we already wrote it to the newly created MVMConcBlockingQueueNode's value but before we add that node to the queue. Move the assignment down to match push's implementation which does it right.
11:44
MoarVM: f18bfadbe8 | (Stefan Seifert)++ | src/spesh/plugin.c
Add missing MVMROOT in MVM_spesh_plugin_register

name and plugin could be moved by the GC before we add them to spesh_plugins
MoarVM: 4ab1d425e1 | (Stefan Seifert)++ | src/core/args.c
Fix autobox possibly writing to an outdated target

Since autobox is a macro, the target argument is not evaluated at the place of the call but in the expanded code. That happens only after allocating, so the GC may have already moved the target frame. Avoid this by forgoing the local variable and accessing tc->cur_frame->caller again instead as that will have been updated.
nine dogbert17: maybe these help with your issue as well?
11:47 robertle joined
dogbert17 nine: will test immediately :) 11:51
nine Though zeroing out the past fromspace helped with uncovering these issues, there is a snag with it: we actually don't free STables during GC, but do so in the next run. I.e. we actually keep using objects from the past fromspace. 11:57
dogbert17 heh 12:01
I wonder if there are any GC related bugs left now 12:06
nine I'd bet there are ;) 12:10
I wonder if it'd still be worth it to memset everything but those STables 12:16
dogbert17 if it makes it easier to find bugs then some kind of option/flag to do that might be handy 12:17
nine Yeah, I figured I would hide that memset behind MVM_GC_DEBUG anyway. So I may as well make it a little more involved 12:18
12:20 ggoebel_ left
dogbert17 unfortunately the bug in t/spec/S12-meta/primitives.t is still present 12:29
probably a good test for your zeroing logic 12:30
12:40 ggoebel_ joined 12:58 brrt joined
brrt \o 13:02
dogbert17 hi brrt 13:16
brrt hi dogbert17 13:22
13:22 brrt` joined 13:26 brrt left
brrt` nine++ fugbixing 13:27
13:32 brrt` left 13:34 lucasb joined 14:06 brrt` joined
nine I now get only a few failures with the zeroed-out fromspace (except for those STables). All of them seem to be related to hashes 14:07
14:13 brrt` left, brrt` joined
dogbert17 interesting 14:21
15:05 brrt` left
nine BigInt operations are a real treasure trove of GC issues! 15:16
MVMP6bigintBody is inlined into MVMP6bigint, so even though it's not an MVMObject itself, it can get moved! While those bigint ops are MVMROOTing the bigint objects, they rarely take care to get the body's new address 15:18
timotimo oh damn 15:20
that's a good one
and sounds rather dangerous 15:21
nine (Poisoning nursery_fromspace)++ 15:22
Geth MoarVM: a001b7e4c7 | (Stefan Seifert)++ | src/math/bigintops.c
Fix many memory corruption issues in bigint operations

MVMP6bigintBody is inlined into MVMP6bigint, so even though it's not an MVMObject itself, it can get moved. While those bigint ops were MVMROOTing the bigint objects, they rarely took care of getting the body's new address.
15:26
timotimo good eye 15:28
bummer we *just* released :)
dogbert17 hello timotimo
I have gotten hold of the information you requested - gist.github.com/dogbert17/81c3aa3a...abf6034b78 15:30
timotimo why does the backtrace look so strange :) 15:31
dogbert17 I had to use some hacks 15:32
timotimo i should have suggested to just break on MVM_gc_enter_from_allocator and use "commands", "bt", "c", "end" 15:33
and then "set pagination off"
then it'd just have given a gdb backtrace for every gc run 15:34
and you would just have had to look at the last one
dogbert17 my suspicions goes toward call_resolver - github.com/MoarVM/MoarVM/blob/mast...gin.c#L401
clever idea
it calls setup_for_guard_recording can trigger a GC so what happens then to plugin_guards stuff? 15:36
*which can trigger 15:37
timotimo i think the plugin_guards thing isn't a gc-moveable, it just contains a bunch of pointers that can move inside of it 15:39
and the tc roots should take care of that
but i can still overlook something
i don't know what MVM_frame_special_return does exactly, can it trigger gc?
nine IIRC it cannot 15:41
timotimo in that case that function could be fine
dogbert17 at the start of call_resolver tc->plugin_guards are saved in some locals which are later in the function assigned to srd->prev_plugin_guards 15:42
I thought it had something to do with that but I'm probably mistaken
timotimo prev_plugin_guards gets rooted for the setup_for_guard_recording call
nine Oh boy.... with poisoning and a tiny nursery one doesn't have to look for bigints to find GC issues. They are there even in basic argument processing
timotimo i think that's the only place in there that can allocate
nine: wow, that sounds promising 15:43
dogbert17 I believe so too
nine params is inlined in MVMFrame, so params can move. It's address is passed to _all_ param processing functions
dogbert17 shouldn't stuff like that crash all the time? 15:44
nine Basically all of args.c is broken
timotimo i think because params is used read-only for most cases, it's usually fine? 15:45
nine dogbert17: it does with the tiny nursery and a poisoned fromspace. Nothing passes anymore
dogbert17 quite the debug tool you have there :) 15:46
"Basically all of args.c is broken" - quote of the day
timotimo definitely still good to fix, in any case 15:52
15:54 domidumont joined 15:59 domidumont left
nine On a closer look, it's not as bad as I feared. Most functions in args.c only access ctx before GC can happen (if it does at all) 16:01
timotimo well, that's good
dogbert17 timotimo: can you explain this, added check code to lines 402 and 413 - gist.github.com/dogbert17/753155d3...14c8b11657 16:13
16:36 ggoebel_ left
timotimo can you check if the GC run there was a minor or major collection? 16:37
if it's a minor collection, it's possible that the data is only reachable via the gen2, so a missing gen2 root could be the issue 16:41
if it's a minor collection, then there's a missing path via marking
the valgrind gdbserver thingie has a "find pointers to this address" command
that could be very interesting
nine Oh, capturenamedshash actually uses MVM_args_slurpy_named, too and here ctx is not just tc->cur_frame->params 16:46
timotimo yeah it's for when you get an actual capture object with something like nqp::usecapture 16:47
i think? 16:48
nine Ah, so much passing :) 16:53
MasterDuke nine: did you see my comment on github.com/MoarVM/MoarVM/commit/a001b7e4c7 ? 17:00
timotimo bb is used a few lines lower, though 17:05
for the quick path "return a"
nine MasterDuke: just answered 17:06
MasterDuke nine, timotimo: ah, but the `bb = get_bigint_body(tc, b);` needs to be repeated because 580 could have caused a GC and bb to move? 17:08
*b to move
nine yes 17:10
Geth MoarVM: 64a6f69c92 | (Stefan Seifert)++ | src/core/exceptions.c
Fix possible invalid memory access when throwing exceptions
17:11
17:17 domidumont joined
Geth MoarVM: e78080f67d | (Stefan Seifert)++ | src/strings/ops.c
Fix possible access to fromspace in final_strand_match_with_repetition_count

string_from_strand_at_index will allocate, so a and b may have moved even while the arguments to MVM_string_equal were evaluated.
17:22
nine dogbert17: you worried about running out of GC issues? :D 17:23
sena_kun nine++ # so many awesome fixes 17:25
Geth MoarVM: e0bc6e2f1c | (Stefan Seifert)++ | 4 files
Fix slurpy arg handling possibly accessing an outdated arg context

The arg context is inlined into the frame, so even though its not an MVMCollectable itself it may move during GC.
MVM_args_slurpy_named has 2 callers. For param_sn we want to access tc->cur_frame->params directly as that's gonna be up to date. The call capture in capturenamedshash does not inline the arg context, so we can use what we get passed without problems.
17:35
nine There's quite a bit more to fix. Alas, dinner's almost ready and I promised a board game evening to my wife, so those fixes will have to wait. 17:40
dogbert17 nine++, awesome fixes
timotimo: it's a minor collection 17:48
19:04 domidumont left 19:05 domidumont joined 19:25 domidumont left
Kaiepi how does MVM_UNLIKELY work? just ran into a case where code didn't work without it, but does with it 20:00
jnthn That shouldn't happen... It's just a compiler hint about whether the branch is likely to be taken or not 20:01
Kaiepi oh nvm, i was expecting it to invert whatever you passed to it 20:06
when are you supposed to use MVM_UNLIKELY? 20:29
timotimo when the code is hot and you're certain you know the outcome in the vast majority of cases
usually type checks that end in an exception are a good place to put an UNLIKELY 20:32
Kaiepi wdym by the code is hot? 20:36
timotimo is potentially executed lots of times per second
it's not worth much to put an MVM_UNLIKELY in a spot that'll only be run once a second or so 20:37
Kaiepi oh, ok
thanks
Geth MoarVM: 6730e51f92 | (Stefan Seifert)++ | src/strings/ops.c
Fix possible access to fromspace in knuth_morris_pratt_string_index

collapse_strands may allocate, so we need to keep Haystack updated
20:47
20:50 chloekek left
Geth MoarVM: 07f6305f45 | (Stefan Seifert)++ | src/strings/ops.c
Fix possible access to fromspace in string_index_ignore_case

We already correctly root Haystack but were assigning it to Hs_or_gic before that, so that one could have been outdated. Move Hs_or_gic's initialization to a place where Haystack is already stable.
21:05
MoarVM: ca9d44f2ea | (Stefan Seifert)++ | src/6model/reprconv.c
Fix possible access to fromspace in MVM_repr_dimensions

Allocating the result array can cause GC which may move obj, so move the allocation after the call to the repr function. We don't need it that early anyway.
21:08
timotimo anything in MVM_hll_map that could have been fixed recently?
nine not by me 21:09
timotimo the moarvm heapanalyzer is able to b0rk with my new format stuff 21:12
huh, it's trying to read from a CArray via a nativeref after the carray has already been GC'd 21:18
that's not good
and valgrind itself apparently segfaulted, too 21:22
21:25 squashable6 left 21:27 squashable6 joined 22:17 squashable6 left
Geth MoarVM: 8f425147a8 | (Samantha McVey)++ | 4 files
Update Unicode to 12.1
22:18
lizmat samcv++ 22:19
22:21 squashable6 joined 22:22 squashable6 left, squashable6 joined 22:28 squashable6 left 22:29 squashable6 joined
MasterDuke i have a rough translation of the Barrett stringification code into non-optimized C 22:57
it's still the recursive version with some string manipulation, but perf says the vast majority of the time is spent in libtommath multiplying numbers, so it doesn't seem like an optimized version would make a great difference 22:59
to stringify 2**1_000_000, rakudo right now takes 57s, a pure perl6 version of the algorithm takes 3.3s, and the C version takes 2.7s 23:01
timotimo a million, that's pretty far out there 23:03
when do they break even?
MasterDuke pretty low. i can't measure too precisely, since there is some overhead in my timing of plain ~, but the C version seems just as fast even for tiny numbers, e.g., 2**6 23:08
timotimo brilliant 23:10
MasterDuke timotimo: gist.github.com/MasterDuke17/95699...6a00d95ac2 23:12
timotimo i wonder if the code could sensibly re-use mp_int structs 23:13
i don't see any function calls to free mp_int structs 23:14
MasterDuke the code would need to be cleaned up and made to handle negative numbers for sure before it's used, but yeah, there are some optimizations to be had 23:15
timotimo unfortunately, i'm not sure that's an optimization :) :) 23:16
MasterDuke like you and the guy in the mailing list mentioned, a non-recursive version would make sense
timotimo i don't claim to have come up with that :) 23:18
jnthn is happy to see all the nice work going on :) 23:27
timotimo so right after the release we'll have to tell people "oh yeah go with latest master moarvm, the released one is full of crashes" :P 23:28
to be fair, the crashes mostly need a fair amount of artificial pressure to be applied to the GC
AlexDaniel jnthn: obligatory ping: github.com/rakudo/rakudo/issues/30...-515720444 :)
although that's a pretty old bug :) 23:29
timotimo: speaking of the releaseā€¦
what's up with github.com/rakudo/rakudo/issues/3063 ?
some people can reproduce it, some can't. I'm not even sure what the right steps are 23:30
timotimo what the heck?
jnthn AlexDaniel: huh, that's...a weird one, since it's just adding debug names, I think?
timotimo but turn on RAKUDO_MODULE_DEBUG or what it's called
that could be interesting
jnthn AlexDaniel: I'm in theory on vacation next week, and then it's PerlCon the week after, so I'm not sure I'll get to investigate it terribly soon. 23:31
timotimo oh
the last comment in there has something interesting to say
no clue what's up with that 23:32
AlexDaniel jnthn: yeah, could it be this? github.com/perl6/nqp/commit/a5232c...1f31b91057
timotimo where do all the "omfg not a git repo" outputs come from
AlexDaniel that's the only commit that does somethingā€¦
timotimo is this a difference between from git clone and from release tarball?
AlexDaniel but I did confirm that that's the right bump:
c: 34942c8597d^,34942c8597d use lib "sandbox/for-perl6-bug/lib"; my $aplugin = do { require ::("APlugin"); ::("APlugin"); }; say $aplugin.new.trycall();
committable6 AlexDaniel, gist.github.com/3fdff0b768e86aa0ab...069ddd4baf
AlexDaniel timotimo: yes, and it's fixed for the point release 23:33
timotimo ok cool
jnthn AlexDaniel: Feasibly, but it's odd nothing would have tripped over it before now if it were wrong...that's an extremely common code path.
MasterDuke heh. even the C version took 6m11s to stringify 2**10_000_000 (which, if anyone is curious, has 3,010,300 digits) 23:35
jnthn And I've certainly got no idea what to do about it if it is to blame without having a much closer look
timotimo i assume the previous implementation we had would take upwards of an hour for that? ;)
MasterDuke i'm not going to punish my laptop by even trying 23:36
AlexDaniel MasterDuke: what about trying the same in other languages? :) 23:37
well, not that large one, but simply benchmarking 23:38
timotimo does the algorithm in question parallelize at all? 23:43
it recurses in two parts, which sounds like "yes, in theory it could" 23:44
but who knows what the overhead would be
MasterDuke for 2**300000, perl 5 takes 6.2s with 'use bigint', perl 6 takes 5.3s, python(2|3) takes 0.2s, ruby takes 0.07s 23:46
timotimo that perl 6 figure is with what we have in master? 23:47
MasterDuke yeah
AlexDaniel so what do they use in ruby? :) 23:48
MasterDuke for 2**1000000, python(2|3) takes 1.5s, ruby takes 0.1s
timotimo should just Proc::Async ruby if it's installed :P 23:49
MasterDuke so they're significantly faster than even my C version of the Barrett algorithm, which takes 2.7s
timotimo does perf point out what functions it's spending its time in?
MasterDuke i've just been counting chars, not printing them. maybe they have an optimization for that 23:50
timotimo ah, possible
MasterDuke timotimo:
timotimo can try getting the first and last 10 chars or something
that'd be unlikely to be optimized a lot
MasterDuke timotimo: s_mp_mul_digs
82% 23:51
timotimo that's for ruby?
MasterDuke no, my c coe
*code
for ruby the top is __gmpn_mul_basecase, at 12% 23:52
heh, for python3 it's 0x00000000001c2f30 at 12% and 0x00000000001c2f29 at 11% 23:54
timotimo hm, is that gnu gmp?
MasterDuke for ruby? i would assume so
bugs.ruby-lang.org/issues/8796 23:55
timotimo FWIW, if i'm looking correctly, getting only the first n chars of a biiiiig number should be pretty fast with barett?
MasterDuke i think so 23:57
timotimo i wonder if that ever comes up in user code, like at all
AlexDaniel ah crap 23:58
I think rakudo fails to configure if you have ( in its path 23:59