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 | |
*thing | |||
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. |
14:30 | |
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. |
14:35 | ||
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. |
14:39 | ||
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 |
14:43 | |
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 | ||
eh | |||
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 | |||
*has | |||
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'? | |||
to: | |||
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'? True alive 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 | ||
jnthnwrthngtn | .oO( Just order pizza and keep hacking! ) |
17:51 | |
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, | ||
'MVM_OPERAND_INT8', 8, | |||
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 |