Welcome to the main channel on the development of MoarVM, a virtual machine for NQP and Rakudo (moarvm.org). This channel is being logged for historical purposes.
Set by lizmat on 24 May 2021.
00:02 reportable6 left, reportable6 joined 00:54 frost joined 03:18 squashable6 left 04:41 jnthnwrthngtn left, rypervenche left, jnthnwrthngtn joined, rypervenche joined, rypervenche left, rypervenche joined 04:45 ilogger2 left, ilogger2 joined 05:30 squashable6 joined 06:02 reportable6 left 06:05 reportable6 joined 06:16 nebuchadnezzar joined
nine MasterDuke: no, I've tried linking in libSegfault, but that doesn't seem to have done anything 07:26
07:44 linkable6 left, evalable6 left 07:46 linkable6 joined
nine Further investigation showed that the JIT _was_ doing it right after all. I.e. it always knows the actual current position. No need to add any additional code to maintain the current position. 08:32
Why did it fail then? Because of yet another, different bug. It's an off-by-one error. The JIT considered the instruction immediately following an inlined region to still be part of that region. That's why the frame walker walked the inlinee's outer chain and didn't find the lexical in question. 08:33
I.e. it's the exact opposite of the non-JIT-code issue.
Btw t/spec/S17-promise/start.t seems to do some unsafe hash access: MoarVM oops: MVM_str_hash_entry_size called with a stale hashtable pointer 08:34
Will commit and push my fixes in the evening 08:35
08:45 frost left 08:47 evalable6 joined 09:09 patrickb joined
jnthnwrthngtn morning o/ 09:41
Nicholas \o 09:43
tellable6 2021-08-29T20:07:38Z #moarvm <MasterDuke68> Nicholas did you see my question on github.com/MoarVM/MoarVM/pull/1481 ?
09:55 patrickb left 10:11 frost joined
jnthnwrthngtn At least one source of the "nextsame is not in the dynamic scope of a dispatcher" seems to be thanks to NQP's method dispatcher having no idea about callsame, which in turn means that if you mix a role into a meta-object implemented in NQP then you'll not having a working nextsame in there 10:50
10:51 patrickb joined 10:56 rakuUser joined 11:04 nebuchadnezzar left, nebuchad` joined
jnthnwrthngtn Method::Also does it, which is in turn used by at least LibXML and probably a bunch of other things 11:05
11:09 nebuchad` is now known as nebuchadnezzar
lizmat jnthnwrthngtn: perhaps a less intrusive way of making allowing aliases for method names would be a better solution? 11:20
I'm thinking a specialized dispatch program ?
jnthnwrthngtn lizmat: I highly doubt Method::Also is the only thing that relies on this 11:21
lizmat well, true: but maybe most of the uses are trying to achieve the same tihng? 11:22
which is why I made that module in the first place: to have a HLLy way of aliasing methods
jnthnwrthngtn I suspect many different MOP-using modules would run into it 11:23
So it needs fixing in general
11:23 Geth joined 11:24 sena_kun joined
lizmat Weird, the logger timed out, but Geth didn't, yet they live on the same machine 11:24
I guess they had different IRC servers 11:25
jnthnwrthngtn Got Method::Also working again (with a general fix) 11:34
Will see what spectest makes of the change 11:36
sena_kun jnthnwrthngtn, do you want a Blin run maybe? 11:39
jnthnwrthngtn sena_kun: I may first look at another issue afflicting LibXML, as that may also fix more modules 11:44
sena_kun jnthnwrthngtn, alright, ping me anytime for a run. 11:45
11:47 patrickb left 12:02 reportable6 left 12:28 patrickbkr joined 12:48 frost left 13:03 reportable6 joined
lizmat And yet another Rakudo Weekly News hits the Net: rakudoweekly.blog/2021/08/30/2021-...perseding/ 13:30
jnthnwrthngtn "failry hot" :D 13:31
lizmat jnthnwrthngtn++ # fixed
jnthnwrthngtn lizmat++ 13:32
Lot of module updates this week :)
lizmat yup, BTW, do you recognize the picture ? 13:34
jnthnwrthngtn It...looks somehow familiar 13:36
lizmat we've had dinner there at least once :-) 13:37
jnthnwrthngtn Does it have outside seating too? Wondering if we cycled there together once also. 13:40
lizmat yeah, we did... 13:41
sadly they will be closing by the end of Sep :-( 13:42
jnthnwrthngtn :-( 13:47
lizmat yeah... :-( :-( 14:04
Geth MoarVM/new-disp: 4e84cd01fe | (Stefan Seifert)++ | src/jit/graph.c
Fix off-by-one error when JIT determines end of inlinee

The JIT treated the instruction immediately following an inlined range as part of that range. This made e.g. the frame walker walk the wrong chain, when looking through the outer lexical scope.
Fix by adding the label before the instruction carrying the MVM_SPESH_ANN_INLINE_END annotation, instead of after it.
MoarVM/new-disp: 9a9ac6991d | (Stefan Seifert)++ | src/spesh/deopt.c
Fix frame walker using wrong address for inline check if not JITed

MVM_spesh_deopt_find_inactive_frame_deopt_idx did not take the actual current position in the currently executing frame but the one of the last invocation. This caused the frame walker to wrongfully believe we're still in an inline and traverse the wrong outer chain. Fix by special casing tc->cur_frame to take the address of the current op instead.
MoarVM/new-disp: 1760a7b2db | (Stefan Seifert)++ | 3 files
Fix MVM_frame_find_lexical_by_name not taking inlines into account

Spesh may inline a callee from a completely different lexical scope into the caller. MVM_frame_find_lexical_by_name however just used the caller as a starting point and followed its outer chain to find a lexical. This caused us to miss lexicals that are only visible in the inlinee's lexical scope.
Fix by replacing the home grown lexical chain walking with the frame walker, which knows all about the transformations spesh does.
nine jnthnwrthngtn: a look at these ^^^ would be greatly appreciated 14:40
Geth MoarVM/new-disp: ef8fe2e0dd | (Jonathan Worthington)++ | src/core/nativecall_dyncall.c
Re-allow passing nulls to native callbacks
jnthnwrthngtn sena_kun: blin! 14:44
sena_kun jnthnwrthngtn, aye
jnthnwrthngtn nine: I'm a little surprised we got away with 4e84cd01fe for so long... 14:45
That one look good to me
sena_kun jnthnwrthngtn, am I correct it takes care of the NextsameIsNotInTheDynamicScope / Callsame... errors?
nine jnthnwrthngtn: I'm a little surprised we got away with any of them TBH :D 14:46
jnthnwrthngtn sena_kun: I don't know; that's partly what I hope to find out. I know it takes care of *a* cause of them. I'm not sure if it's the only cause. 14:47
(But if there are others, I don't know what they are.)
sena_kun aye, let's find out!
Commit exists, but an executable could not be built for it 14:48
jnthnwrthngtn Odd
sena_kun ah, it wants a bump
jnthnwrthngtn ah :) 14:49
sena_kun mvm and nqp one...
jnthnwrthngtn nine: The second one looks sensible to me also
sena_kun I'll do, please take care to push with --rebase to not get a mess when I taint the holy commit tree... 14:50
jnthnwrthngtn Hm, are you rebaising the branches as part of adding the bumps, or they are just normal commits?
sena_kun they are just normal commits 14:51
jnthnwrthngtn ah, ok, then fine :)
sena_kun let's see if it helps... 14:54
15:14 evalable6 left, linkable6 left 15:15 linkable6 joined 15:16 evalable6 joined
nine About the mis-compilation in bug-coverage.t: what's hanging is sub (uint8 $x) { Nil until $x }(2); Any unsigned int type will cause a hang, even uint64 where we don't emit a truncate. Signed ints won't cause a hang, but they are still compiled wrong (if < 64 bit), because while we do emit an appropriate trunc_* op, we don't actually use its result 15:45
We just work with the original value
jnthnwrthngtn nine: Ah yay, you're looking at that one :) 15:46
nine I figured, it can't be worse than the bug minefield that the MVM_frame_find_lexical_by_name issue was :D 15:47
jnthnwrthngtn Fair point :) 15:49
m: class T { }; class S is T { }; multi foo(|c(S $a, T $b)) { 1 }; multi foo(|c(T $a, S $b)) { 2 }; multi bar(|c(S $a;; T $b)) { 1 }; multi bar(|c(T $a;; S $b)) { 2 }; foo(S,S)
camelia ( no output )
jnthnwrthngtn Wait what 15:50
m: use Test; class T { }; class S is T { }; multi foo(|c(S $a, T $b)) { 1 }; multi foo(|c(T $a, S $b)) { 2 }; my $lived = 0; try { foo(S,S); $lived = 1 }; is($lived, 0, "dispatch tied as expected"); 15:52
camelia not ok 1 - dispatch tied as expected
# Failed test 'dispatch tied as expected'
# at <tmp> line 1
# expected: '0'
# got: '1'
jnthnwrthngtn But...that test apparently passes on master...
oh goodness, it passes because a totally unrelated earlier test as
class A { } 15:53
class B { }
proto foo(|c($x)) { * } #OK not used
And we currently fail to enforce non-trivial protos
sena_kun while I am still running blin (the results are looking pretty nice so far, though the worst usually comes near the end due to bisecting), but note AttrX::Mooish leaks like crazy, ate 30g of RAM at ease before I sigterm it. 15:55
jnthnwrthngtn Leak or just some kind of infinite recursion perhaps?
sena_kun alas, no idea 15:56
perhaps some sort of infinite recursion, as it was not going to stop soon from what I saw, just ate 1G in a second 15:57
timo do we already use a dispatcher for .isa(Str:D) and .HOW.name, i wonder? i imagine that would work well. type known -> return constant string for one of the two, the other could "specialize" on a constant string argument, the .isa one i mean
i wonder if in a new-disp world json::fast can be faster with a multi method to dispatch to the right piece of code for serialization rather than a big if / elsif with nqp::istype in it 15:58
dogbert11 oops, a SEGV 16:15
moar: src/spesh/deopt.c:32: uninline: Assertion `MVM_callstack_current_frame(tc) == f' failed. 16:18
to repro: MVM_SPESH_NODELAY=1 ./rakudo-m -Ilib t/spec/S32-io/io-cathandle.t (I have set the nursery size to 48k) 16:19
16:26 patrickbkr left
jnthnwrthngtn m: proto x($ where * > 0) { {*} }; multi x($i) { say $i }; x(1); x(-1); 16:36
camelia 1
Constraint type check failed in binding to parameter '<anon>'; expected anonymous constraint to be met but got Int (-1)
in sub x at <tmp> line 1
in block <unit> at <tmp> line 1
timo ah 16:58
ww :)
sena_kun Blin run is almost finished, I guess about 40 modules is what we have until next time. good news: 40 is less than what we had, no more callsame errors and only one module remains with `nextsame is not in the dynamic scope`. the rest of the picture is similar to what we had. 17:16
one actually needs to go through the list and check what are genuine bugs and what are expected breakage or weird flappers to present a better set of results 17:17
jnthnwrthngtn What did we have last run? 17:18
sena_kun let's see...
57 17:19
almost a third is fixed, yes
jnthnwrthngtn Which one is the remaining nextsame not in dynamic scope one? 17:20
sena_kun PDF::Class
Note it can just rely on other PDF-related modules, because others have errors as well. PDF::API6 and PDF::Content have an error related to argument flattening.
So I would see if those can e fixed first, especially given there are 5 modules in total with this error. 17:21
Argument flattening error also causes the most amount of breakage (5 modules) as a reason, the next reasons are 3, 2 and then it's all horror. 17:23
jnthnwrthngtn Good news: t/spec/S06-multi/subsignature.rakudo.moar is now fixed 17:24
That gets us down to 4 failing spectests 17:25
sena_kun fantastic
jnthnwrthngtn Well, spectest files 17:26
And nine++ is on with one of those, so probably 3 soon :)
sena_kun Oh, disregard stupid me, PDF::API6 and the other one depend on PDF::Class, so no point to look at them before PDF::Class maybe.
jnthnwrthngtn sena_kun: Did the PDF module itself install successfully? 17:33
Also, was this run any faster than the previous new-disp ones?
nine nqp: sub foo(uint8 $i) { Nil until $i }; foo(2); 17:34
camelia (timeout)
nine Intruigingly it doesn't actually seem to be parameter handling but loop condition handling 17:35
jnthnwrthngtn nine: I think that it somewhere wants to widen the value so it can feed it to if_i (or unless_i), and it's that widening coercion that goes bad 17:36
sena_kun about 2 hours in total, but I don't think it's representative yet, it fluctuates a lot, e.g. at one point the RAM eater was keeping busy 17 cores.
nine exactly
The @Full-width-coerce-to thing
sena_kun jnthnwrthngtn, I can suggest you a smaller module with the argument flattening error, maybe it'll be easier to investigate that. 17:37
hmm, no, it seems I can't, HTML::Canvas is large as well. 17:38
jnthnwrthngtn What is the error? The callsite record size one?
sena_kun this needs manual filtering, I think
Argument flattening array must be a concrete VMArray 17:39
jnthnwrthngtn ohh
That's...a surprise
Example of a mdoule giving that?
sena_kun a large one is HTML::Canvas
PDF::Content 17:40
jnthnwrthngtn Thanks, probably hometime soon but will look at those later 17:41
sena_kun anyway, there are a lot of them with super specific errors and I am not sure what would be the best spot to look at
also some are cases of "It gave error A on master, but gives error B on new-disp".
jnthnwrthngtn As in wrong type of exception, or? 17:42
Or modules that failed to test/install but just changed failure mode?
sena_kun no, just a completely different error
not a wrong type of exception from what I see
jnthnwrthngtn So the tests fail 'cus it was looking for a particular error message? 17:43
sena_kun if it was just this easy
E.g. `Colorizable` module went from:
❌ TST: Applying '.contains' to a Map will look at its .Str representation.
❌ TST: Did you mean 'Map{needle}:exists'?
tellable6 sena_kun, I cannot recognize this command. See wiki for some examples: github.com/Raku/whateverable/wiki/Tellable
sena_kun ❌ TST: Argument flattening array must be a concrete VMArray 17:44
jnthnwrthngtn Oh. That sounds like warning to error
nine jnthnwrthngtn: we're literally missing the whole code to do the actual coercion. All we do for loop conditions is allocate a coerce register and then just pretend it contains a value :D
jnthnwrthngtn m: say { a => 42 }.contains('a'); say "alive";
camelia Applying '.contains' to a Hash will look at its .Str representation.
Did you mean 'Hash{needle}:exists'?
in block <unit> at <tmp> line 1
jnthnwrthngtn Yeah, just a warning before, so we broke it 17:45
Hmmm, this is odd. PDF::Class passes for me locally 17:46
sena_kun which version?
jnthnwrthngtn I just cloned the github repo
So `master`... :)
sena_kun ah
well, yes, as I wrote this list needs manual filtering to narrow down where are real cases and where are just oddities 17:47
jnthnwrthngtn Sure. Was just curious to repro the one remaining case of the thing I fixed all the other cases of today, is all. No worries, will get back to it. 17:49
sena_kun phew, alright
don't push yourself too hard. :) 17:50
jnthnwrthngtn I think I'm going to push myself home now
There's cooking to be done
sena_kun wise choice, yes
.oO( Just order pizza and keep hacking! )
nine Got a fix for the infinite loop, but get an "operand type 32 does not match register type 8 for op param_rp_i in frame" in a different place now. Will investigate whether it's just yet another issue or if I made a mistake
timo 32 is such a high number, is that just one of the sized integers? 17:56
jnthnwrthngtn bbl o/
nine 'MVM_OPERAND_INT64', 32,
timo huh
17:57 sena_kun left
nine Easy fix: just get rid of that validation check. Works out just fine ;) 18:01
timo aha
18:02 reportable6 left
Nicholas I appreciate your "optimisim" - "then it's all horror" 18:08
nine What the? Fixing the loop code gen causes a retroactive change to the register type for the param_rp_i op 18:09
timo we get that kind of problem when we put a "has" with a native type after where it's used for the first time as well, don't we? 18:21
i think there it's obj vs whatever type you got 18:22
nine Passing $new=1 to $regalloc.fresh_register fixes that one 18:23
Ah, no, it doesn't. I just had that validator commented out still 18:24
# set $new to 1 here if you suspect a problem with the allocator 18:30
That does help
But I think it's not actually the register allocator that's wrong. The retroactive part comes from us generating the code for parameters after the actual body of the block. I remember the headaches this gave me when writing the new mbc backend 18:32
Ah, yes, I just forgot to adjust the release_register call. So I released the wrong register, which causes all sorts of fun of course :) 18:36
With that taken care of bug-coverage.t passes just fine :)
jnthnwrthngtn: fix pushed to NQP. We are indeed down to just 3 failing spec test files :) 18:45
timo ooh
dogbert11 nine++ jnthnwrthngtn++ so three spectests and a SEGV left, very impressive 18:58
19:05 reportable6 joined
dogbert11 ==1055183== Invalid read of size 8 19:07
==1055183== at 0x4A5EEC4: MVM_interp_run (interp.c:5481)
==1055183== by 0x1097EF: main (main.c:305)
==1055183== Address 0x8036a2d08 is not stack'd, malloc'd or (recently) free'd
MasterDuke gist.github.com/MasterDuke17/21403...670f8abd60 has the invalid free and a bunch of leaks from running `valgrind --leak-check=full nqp-m --full-cleanup -e ''` on new-disp 19:34
timo yeahs, i think those missed frees are from having some dispatch programs that aren't freed 19:42
i fixed a part of those, but that only hit a subset
19:44 patrickb joined 19:54 patrickb left 19:57 discord-raku-bot left, discord-raku-bot joined
jnthnwrthngtn nine: Hurrah, a non-hanging spectest run :D 20:48
timo i want a boatload of silly, random stats about how spesh uses its memory. is there anything to be gained from stuff like re-organizing the instructions and/or operator arrays after a specific stage so that iteration can go much faster? any reason to specialcase "the next/previous struct in this linked list sits in the next/previous slot in memory" so we have something simpler than following a 21:01
pointer for prefetching purposes?
are there ops that we pretty much always turn into an op that has one or two more operands and we could actually allocate the operands array a little bigger at the start and re-use that during the transform?
my brain is throwing useless ideas at me to see what sticks 21:02
would there be a benefit to sometimes having opcode + operand count where the MVMSpeshIns currently holds the info pointer? do we grab more info from the info struct than those two in many cases at all? perhaps having a tight array of operand counts with perhaps 6 bits per entry? 21:04
jnthnwrthngtn Hmm....sometime recently we've lost a lot of the gains on CORE.setting compilation time again 21:59
We were down to less CPU cycles than `master` (322 billion), and very close on wallclock. I saw the wallclock was up again but figured my machine maybe was busy, so callgrind'd it while away, and it's up to 360 million again 22:01
Hm, yup, a bunch more frame invocations 22:02
From 89 million to 96 million 22:03
Hm, and we actually seem to do more inlines, not less, so it's really that there's more frames 22:04
22:33 dogbert11 left
MasterDuke timo: yeah, dramatically fewer leaks after your fixes, but not yet to zero 22:38
jnthnwrthngtn Wow, apparently it's 6d62a2cb40 22:41
(The HEAD commit in Rakudo)
That's a bit of a surprise.
23:01 dogbert11 joined
jnthnwrthngtn Well, figured it out and restored the setting compilation performance. Now for sleep. 23:31