github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
nwc10 good *, #moarvm 06:10
MasterDuke interesting, this segv on my use_fsa_for_vmarray branch also has nqp::create and allocate() in the backtrace, same as that segv from the use statements i was just attempting to debug 09:09
jnthn MasterDuke: That's not terribly surprising, in so far as `create` is one of the most common paths into the GC (assuming the crash is in the GC) 10:32
MasterDuke huh. it's just that i haven't seen it much before. guess most of my previous experiences weren't gc-related 10:34
fwiw, i'm getting a `free(): invalid pointer` in MVM_fixed_size_safepoint when running t/spec/S32-io/io-cathandle.rakudo.moar 10:44
hm, but the FSA shouldn't be freeing something that was created by the gc, right? 11:20
doh, nm, that's how freeing at a safepoint is handled 11:21
jnthn Any luck with valgrind/ASAN, since it's potentially a double free? 11:33
MasterDuke you know, i'd completely forgotten about the valgrind run i did gist.github.com/MasterDuke17/f1024...5acc62fd77 11:38
it goes away with GC_DEBUG=3 11:40
jnthn oh, that's interesting output
Did you try the FSA debug thing?
MasterDuke doh, again...let's give that a run... 11:41
jnthn Because it looks like it is handing pack memory inside of a page (so the initial request was small), but then freeing as if it was an oversized page
e.g. the size is being provided incorrectly at freeing time
*handing back
MasterDuke yeah
jnthn And FSA debug can detect those 11:42
Could be something like accidentally multiplying wrong at free time, or using the new size instead of the old size when doing the free because of a growth
MasterDuke hm, insta segv 11:43
jnthn Wow 11:47
MasterDuke oh, the fsa doesn't like to do anything (alloc, free, etc) with a size of 0, right
jnthn Can't remember 11:49
MasterDuke yep, github.com/MoarVM/MoarVM/pull/689/...dca5cdR107 `arr->body.ssize` was 0 11:50
jnthn Ah
MasterDuke woohoo, spectest now passes 12:03
jnthn Yay :) 12:04
MasterDuke jnthn: however `my @a; race for (^10_000_000).race(:100batch, :8degree) { @a.push($_) }; say @a.elems; say @a.tail` still segvs. i sort of thought it now *should* just give "wrong" values 12:06
jnthn Well, did the re-org happen to move the ssize and so on into the thing allocated via the FSA? 12:08
(And the start too, I guess)
MasterDuke you know, i've forgotten (haven't looked at the early commits recently). but i suspect not 12:09
i think it's just slots that are fsa now 12:10
guess that'll be the next step 12:11
timotimo right, putting only the slots in the FSA is not yet enough 12:17
MasterDuke any particular suggestions for the best way to stick the other stuff in? 12:36
timotimo not sure if you can automate the transformation 12:40
jnthn I guess it's 2 or 3 items and that they are pointer sized; I guess I'd try and keep the amounts of memory we ask for as powers of 2 (at least up to a certain size) 13:22
It's a bit annoying 13:23
MasterDuke i assume it'd be too simple to "just" make body a pointer and FSA alloc it? github.com/MoarVM/MoarVM/blob/mast...rray.h#L41 14:15
(with appropriate changes to how it's accessed of course) 14:16
jnthn Hm, well...maybe but then we have two allocations with the FSA and I think it's easier to reason about the safety if there's one
I mean, we want one self-describing piece of memory 14:17
nwc10 I think that you can prove the concept with two blocks of memory. But then really it should be optimised to one 14:19
it also makes the CPU cache performance better
jnthn In terms of "doesn't segfault" I think it's much harder to prove :) 14:20
MasterDuke you mean for body and slots? well, i think i was thinking of doing it all in one go and adapting the current places that (re)alloc/free slots to instead adjust the whole piece
nwc10 by prove the concept - I meant get the current spectests passing
ah OK. If you think all in one go is easer, go for that
jnthn nwc10: Ah, I see.
lizmat and another Rakudo Weekly News hits the Net: rakudoweekly.blog/2020/11/23/2020-...t-release/ 19:37