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 00:03 reportable6 joined 01:08 frost joined 03:04 bisectable6 left, statisfiable6 left, unicodable6 left, evalable6 left, benchable6 left, greppable6 left, bloatable6 left, squashable6 left, tellable6 left, linkable6 left, coverable6 left, nativecallable6 left, reportable6 left, committable6 left, notable6 left, sourceable6 left, shareable6 left, quotable6 left, releasable6 left 03:05 shareable6 joined, bisectable6 joined, bloatable6 joined 03:07 reportable6 joined, statisfiable6 joined, squashable6 joined, quotable6 joined, benchable6 joined, nativecallable6 joined 04:05 coverable6 joined, sourceable6 joined 04:06 notable6 joined, committable6 joined 04:07 unicodable6 joined 05:06 releasable6 joined 06:02 reportable6 left 06:05 tellable6 joined 06:07 greppable6 joined, linkable6 joined 06:08 evalable6 joined 07:18 evalable6 left, linkable6 left, linkable6 joined 07:42 sena_kun joined 07:47 patrickb joined 08:02 patrickb left 08:04 reportable6 joined
Nicholas good *, #moarvm 08:06
japhb good *, Nicholas 08:09
Except for me * is bedtime. :-)
Nicholas nine: did you attempt the libffi "port", or should I start on it? 08:38
nine Nicholas: I can do it earliest on Friday. So if you wan't to give it a go, I'll happily leave it to you 08:39
Nicholas OK. I'll tell you if I'm partway and stuck
otherwise it's "not started" or you'll see a push 08:40
nine ++Nicholas 08:55
09:18 evalable6 joined
MasterDuke looks like libuv 1.42.0 was released in july. at a very quick skim, didn't notice anything in the changelog that looked relevant to any open issues moarvm has 09:59
lizmat still... would be good to update, no ? 10:07
MasterDuke and libatomic_ops 7.6.12 (we're at 7.6.2) was released in september
jnthnwrthngtn moarning o/ 10:16
Nicholas \o 10:17
jnthnwrthngtn (good to update) there's always the small risk of a breakage from an upgrade too, although with libuv that has been quite rare
MasterDuke yes. especially since there doesn't appear to be anything immediately needed and we still have a bunch of new-disp related fixes/changes lined up, i'd sort of lean toward waiting until after the next release 10:21
lizmat fair enough :-) 10:22
10:29 linkable6 left, evalable6 left 10:32 evalable6 joined
jnthnwrthngtn So, I guess my challenge now is to work out how to eliminate the thunked flag without breaking the CI :) 10:55
(Since my solution to unbust finalizer calls was to not call them when we thunked.)
nine jnthnwrthngtn: what does the thunked flag actually mean? 10:56
jnthnwrthngtn nine: It was intended as a stopgap while stack unwinding was spread between remove_one_frame and the new callstack mechanism. 10:58
nine: Setting it would suppress additional actions (such as running special return handlers) in remove_one_frame in the event that something new was invoked as part of the unwind 10:59
Now there pretty much aren't any such actions.
Geth MoarVM/new-special-return-only: 80894e0161 | (Jonathan Worthington)++ | src/core/frame.c
Stop considering thunked in remove_one_frame

It should no longer matter.
jnthnwrthngtn This step should be totally innocent, but given I've only got CI to guide me, this is going to be very slow and steady. 11:04
nine Do you have an idea why not running finalizers if thunked is set fixes the problems? 11:10
MasterDuke i wonder if it's possible to get something like a docker image of their CI environments so one can try to replicate locally? it wouldn't necessarily help if it's hardware related, but might if it was some other environmental thing
nine Yesterday I've yet again tried to reproduce by slowing down my system during the test run. Nothing at all.
MasterDuke how did you try? running another spectest in the background? i've found that frequently helps 11:11
jnthnwrthngtn nine: Not really. 11:13
nine Multiple TEST_JOBS=80 spectests in the background. Pro tip: when trying this, keep one of them in focus so you can ctrl+c when the system starts swapping heavily. On my first try I simply rebooted after half an hour.
jnthnwrthngtn About the CI: the crash mentions CompUnit. Could it be that the CI has a clean(er) state (e.g. nothing installed) and so takes a different path there, which triggers the bug? 11:14
It's still really odd that this is the *only* thing that seems to trigger it, though
sena_kun++ also suggested to see if Azure has a debug mode thing like Travis, which I'd not thought of 11:15
MasterDuke github.com/marketplace/actions/deb...g-with-ssh maybe 11:16
jnthnwrthngtn I think it'd have to be an Azure thing? 11:17
Since iiuc we run the CI using that, not GitHub actions
Though knowing it is possible with GitHub actions is useful in general :)
MasterDuke but i think actions are just a wrapper around azure? 11:18
jnthnwrthngtn Yes, but I mean, I think to use actions from the GitHub marketplace we'd have to have our CI pipelines specified in terms of GitHub actions overall, not as an azure pipeline? 11:20
MasterDuke could be
jnthnwrthngtn Looks like that first step is going to be innocent 11:24
Alerady have two more lined up
MasterDuke i wonder how locked down their images are? i.e., could you just set up a reverse shell back to your machine? 11:25
probably not allowed (or very safe) 11:26
jnthnwrthngtn Ah, it seems that the CI is based on an ubuntu base image (even named in our azure-pipelines.yaml), so I could try to recreate it that way 11:28
11:29 linkable6 joined
MasterDuke my laptop uses kubuntu (though i just upgraded to 2021.10), i could see if it replicates there 11:29
nine I just tried testing with a rather clean build directory (no t/spec, no ~/.raku), but all I got was a segfault in 15-gh_1202.t's sub process in MVM_str_hash_lvalue_fetch_nocheck ( MVMHash_bind_key)
MasterDuke jnthnwrthngtn: what are the steps to try and replicate? 11:32
jnthnwrthngtn MasterDuke: Take github.com/MoarVM/MoarVM/pull/1581, remove the final commit, build and make test 11:34
Geth MoarVM/new-special-return-only: cbb33ac423 | (Jonathan Worthington)++ | 3 files
Don't set thunked on finalizer run

Since we no longer check the flag in remove_one_frame, it should quite easily be possible to avoid setting it here.
MasterDuke no errors so far (with gcc or clang) 11:52
12:02 reportable6 left 12:05 reportable6 joined
jnthnwrthngtn > 12:08
NMAKE : fatal error U1073: don't know how to make 'gen/nqp-version'
What on earth? :)
MasterDuke that happens randomly in windows ci, i always just restart those 12:09
jnthnwrthngtn sigh 12:10
Anyway, looks like the above push is alright
Only one job still running, plus this failure. I'll probably not re-run it as the problem tended to show up in 50% of the runs when it broke 12:11
Geth MoarVM/new-special-return-only: c79558defc | (Jonathan Worthington)++ | src/core/callstack.c
Try switching to local detection of thunking
jnthnwrthngtn This is the big one, I guess. 12:13
I need lunch... :)
MasterDuke a couple `make m-test` runs have been fine, but i do see a fail in t/spec/S16-unfiled/getpeername.rakudo.moar
and t/spec/S17-supply/syntax-nonblocking-await.t 12:14
jnthnwrthngtn The latter was mentioned as an issue on master anyway 12:16
The first is curious. 12:17
Is it failing repeatably?
MasterDuke ubsan throws a fit when building nqp. `src/6model/serialization.c:1691:21: runtime error: left shift of 4243 by 52 places cannot be represented in type 'long int'` seems like it could be a problem. a *ton* of `runtime error: member access within misaligned address` and `runtime error: load of misaligned address`
well, the spectest is still running. rakudo definitely feels slower than i remember on this computer 12:18
`src/6model/serialization.c:902:5: runtime error: null pointer passed as argument 2, which is declared to never be null` 12:19
jnthnwrthngtn Those are interesting, although surely unrelated to this branch 12:21
I think if you're running it with UBSAN it's going to feel slower? :)
MasterDuke oops, yeah, those are completely unrelated to new-special-return testing 12:22
those were from my desktop
jnthnwrthngtn The shift things maybe don't matter a lot but the null one is a bit concerning
MasterDuke on the laptop, those two fails aren't happening when run by themselves. t/spec/S16-unfiled/getpeername.rakudo.moar is essentially empty 12:23
fwiw, i don't think any of those usban things are new with new-disp, pretty sure i remember seeing them a while ago 12:24
github.com/MoarVM/MoarVM/issues/1431 12:25
nine The null pointer thing looks quite trivial. I bet it's when writer->root.num_repos is 0, i.e. there are no repossessions and we didn't allocate a repossession table at all. 12:26
jnthnwrthngtn oh, lunch, bbl 12:27
13:10 evalable6 left, linkable6 left 13:11 evalable6 joined
jnthnwrthngtn Cool, it's green :) 13:21
.oO( it's not easy being green )
Geth MoarVM/new-special-return-only: 4c29e70279 | (Jonathan Worthington)++ | 5 files
Remove non-local use of thunked flag

So now we only need to track it locally inside of the unwind frame function.
jnthnwrthngtn In theory this one is now just removing dead code, so should be green also.
Then there's two more patches to put back and test, then I'll tidy it up 13:30
Geth MoarVM/new-special-return-only: 30476664e0 | (Jonathan Worthington)++ | 2 files
Move stack frame extras cleanup into unwind

We can free it more eagerly since we know it didn't escape, and can also avoid having to NULL out the extras pointer.
dogbert17 running a spectest with MVM_GC_DEBUG=3 is slooow 14:38
jnthnwrthngtn I can only imagine... 14:42
So there's only one more commit to add back after ^^ and then I'll tidy up and I think it'll be good 14:45
Nicholas This is all very illogical 14:46
jnthnwrthngtn ? 14:54
Geth MoarVM/new-special-return-only: 374be40ae4 | (Jonathan Worthington)++ | 3 files
Make return from JITted frames cheaper

This shaves ~7 CPU instructions per return from a JIT-compiled frame by avoiding duplicate checks that the compiler is seemingly unable to get rid of (indeed, I've never seen a compiler manage to inline the JIT set position before now). Add some assertions to make sure we never diverge from the behavior of the general case.
jnthnwrthngtn Yay. Assuming this one is also fine, we can haz speedup
15:16 patrickb joined
patrickb o/ 15:17
jnthn: I didn't carefully read the backlog. But what's the difference between the original PR and the current state of it? 15:18
nine patrickb: original PR reliably threw errors on CI that none of us could reproduce locally. Hence the new PR with commit by commit pushes to narrow down the problem and find solutions 15:20
jnthnwrthngtn m: say 1.742 / 1.973
camelia 0.882919
jnthnwrthngtn wait, waht 15:21
meh, typo
patrickb That's what I understood. But it seems just putting the same commits one by one back fixed it without actually changing anything. And that seems more like a reason to cry than rejoice...
jnthnwrthngtn m: say 1.742 / 1.873
camelia 0.930059
timo that's still a decent win
nine patrickb: no, no. He found the issue. Was finalizers getting run at inopportune times.
jnthnwrthngtn Typo'd. OK, that's more in line with the improvement level I observed.
timo that's the fibonacci one again?
jnthnwrthngtn timo: Yes 15:22
timo and comparing master vs new-special-return-only?
patrickb Ah, that'st the bit I missed. Thanks!
nine patrickb: github.com/MoarVM/MoarVM/commit/6f...acca669f6b
patrickb reads up on the finalizer commit
nine patrickb: then some commits for reducing the usage of the thunked variable which was only meant as a temporary solution in the branch. But some part of it apparently has to stay 15:23
jnthnwrthngtn I'm still a bit frustrated I can't debug it locally yet, but the mitigation for the finalizer problem doesn't seem to lose us anything.
Or at lesat, not much.
It'd be nice to lose the "did we thunk" flag entirely, but having it local rather than passed through a load of functions is a big improvement. 15:24
And unblocks the even bigger improvements to cleaned up return handling.
nine I'll call that a victory 15:29
jnthnwrthngtn m: say 3.434 / 3.495 15:34
camelia 0.982546
jnthnwrthngtn A more realistic number for the effect in a realistic application (nearly 2% win in Agrammon) 15:35
m: say 3.434 / 3.634 15:36
camelia 0.944964
jnthnwrthngtn And that's against pre-new-disp
Geth MoarVM/new-special-return: 16 commits pushed by (Jonathan Worthington)++
review: github.com/MoarVM/MoarVM/compare/c...28b9b2d366
jnthnwrthngtn That's a tidied history, back on the original PR
Nicholas the "illogical" comment was because I hadn't realised that the problem was found. But I'm still a bit confused as to why it was not reproducable anywhere other than on the CI system 15:41
nine Nicholas: we all are :) 15:42
jnthnwrthngtn My best guess remains that it's related to a code-path in comp unit handling that is taken on the CI but not locally for...whatever reason. 15:44
nine Not many methods have a --> CompUnit:D return signature. Mostly method need which will be involved just the same. 15:48
[Coke] is surprised to find nom branch still a thing. 15:49
jnthnwrthngtn If anybody fancies a little reviewing, github.com/MoarVM/MoarVM/pull/1589 is also there
nine Just approved it 15:52
Will probably have a look at new-special-return tomorrow. Good thing I've already studied quite a few of the commits now :) 15:53
timo github.com/MoarVM/MoarVM/pull/1585 - this one isn't huge and could also be reviewed. got a look from MD already who pointed out a few things 15:56
Geth MoarVM: fcddd1210d | (Jonathan Worthington)++ | src/core/frame.c
Split specialized and unspecialized frame alloc

This allows for us to eliminate more checks than the compiler can, and also seems - at least on some compilers - to make it possible to inline into MVM_frame_dispatch. One further opportunity is that we can fold two calls to memset into one in the common on-callstack allocation case of specialized frames.
MoarVM: 9903a543dc | (Jonathan Worthington)++ | src/core/frame.c
Elide frame initialization check with spesh cand

If we are at the point that we are making a call where we have already identified a specialization of a frame, then clearly that frame has been invoked before. Thus move the frame initialization check into the path of the non-specialized invocation.
MoarVM: 770e82e484 | (Jonathan Worthington)++ (committed using GitHub Web editor) | src/core/frame.c
Merge pull request #1589 from MoarVM/faster-frame-alloc

Some small optimizations for frame allocation
jnthnwrthngtn Will rebase github.com/MoarVM/MoarVM/pull/1581 on master also, so that I can have a single point with all of the call/return related opts so far, so I can do more on that :) 15:59
16:11 linkable6 joined 16:27 patrickb left
Geth MoarVM/new-special-return: 16 commits pushed by (Jonathan Worthington)++
review: github.com/MoarVM/MoarVM/compare/5...8cf69f03a2
MoarVM/new-special-return: 485ca825f8 | (Jonathan Worthington)++ | 3 files
Avoid unused record -> frame translation

remove_one_frame now has no need for the caller frame, so instead of finding it and then NULL-testing it, just return a boolean from the callstack unwind instead. Shaves ~9 CPU instructions per call to remove_one_frame.
jnthnwrthngtn Figure that belongs in the PR too, since it's an obvious cleanup
16:50 patrickb joined
jnthnwrthngtn language class & 17:01
17:11 camelia left, nine left 17:19 camelia joined 17:24 camelia left 18:02 reportable6 left 18:03 nine joined 18:05 nine left 18:06 nine joined 18:08 camelia joined 18:22 dogbert11 joined 18:24 dogbert17 left 18:25 dogbert17 joined 18:27 dogbert11 left 18:32 MasterDuke left 18:34 patrickz joined, patrickb left 18:45 MasterDuke joined 18:46 dogbert11 joined 18:48 dogbert17 left 18:54 squashable6 left 18:55 squashable6 joined 19:02 squashable6 left, squashable6 joined 19:03 squashable6 left 19:12 shareable6 left 19:13 shareable6 joined 19:18 squashable6 joined 20:05 reportable6 joined
Nicholas nine: OK, first SEGV is at b7d40359a1503dda41e8592297d305ea7b1f2ba5 - as best I can tell you can't pass a NULL ffi_type *rtype to ffi_prep_cif 20:24
timo m: say (908822585 + 908914042 + 908879926) * 100 / (906575765 + 906404294 + 906761700) 20:52
camelia 100.25277377814
timo m: say (908822585 + 908914042 + 908879926) R/ (906575765 + 906404294 + 906761700) * 100 20:53
camelia 99.74786355667
timo m: say (908822585 + 908914042 + 908879926) R/ (905987592 + 905776803 + 905675811) * 100 20:56
camelia 99.66345297105
nine Nicholas: but I don't see how that commit could cause that? We're passing body->ffi_ret_type there, which is set during setup of the native callsite. And that setup hasn't changed at all
Nicholas me neither yet. I have a third build in progress - libfii on master 20:57
but this really was "that commit should work because it didn't change the dyncall/ffi specific code, but I'll just check it anyway" 20:58
"yet" might be "I've gone to bed"
valgrind did not show any problems prior to a NULL pointer deference
timo running rakudo -e '' while printing out what ops are most commonly validated showed getcode at 51.6k, push_o at 25.6k, 13.9k const_s, 13.5k capturelex, 10.8k set 20:59
20:59 patrickz left
timo adding a tiny fastpath for getcode that validates the two operands without going through the op info's arguments arrays in a loop 21:01
gave the first improvement, the 99.75%
putting the operands in there as constants got it down to 99.66% 21:02
ideally someone would put a bit of work into an attempt to SIMD the validator as much as that would be possible 21:05
Nicholas nine: I might still randomly vanish into bed, but rr on that commit shows that we hit copy_to in src/6model/reprs/NativeCall.c
and that looks suspiciously like it can't copy enough 21:06
nine Nicholas: ah, yes, of course. I think I've already fixed 5 different things that copy_to neglected to copy 21:11
timo imagine if we could read something about the number of arguments a code has from its opcode ... 21:12
nine timo: the 16 bits ought to give us enough room to encode that. 21:15
timo aye, we "only" have 943 opcodes
but yeah tossing out extops will give us all ze bits
do we split it in half at the moment for extops?
ah, the ext op base is 1024 21:16
nine And the argument count can be part of the op's id. So we only need enough bits for the distinct ops with a certain argument number
MasterDuke would rakuast make it possible to have built-in support for something like `multi foo(either(Int $a, Str $b)) { ... }` instead of `multi foo(Int $a, Str $b) { ... }; multi foo(Str $a, Int $b) { ... }`?
timo that's right. i'll want to count that up real quick
360 2, 21:17
210 3,
116 1,
80 4,
35 5,
11 6,
9 0,
5 7,
4 8,
1 2
1 2 21:18
1 0,
nine So 9 bits leaves plenty of room for new ops with 2 operands. Does the count include deprecated ones?
timo like 72 deprecated ops are in there with different numbers of operands 21:19
very very many of them are 2 args 21:20
only 36 of them
nine Anyway 4 bits for argument count seems to be plenty enough.
timo so not getting below 256 for 2-op ops 21:21
but, if we turn for example all the trigonometry ops into syscalls
also just like 9 of them are obviously rare enough for that purpose 21:22
nine I thought syscalls are more something for rarely used ops?
timo copy_f, append_f, rename_f, chmod_f, exists_f, mkdir, open_dir, read_dir, trunc_fh, those are all 2-op ops that may be rare enough for this purpose 21:24
in jitted code the cost difference between a syscall and an op should be equal, just more space used in the .moarvm bytecode file
if we can make bytecode validation simpler/faster by tossing a few of the rarest ops, that could be a thing we may do 21:25
m: say 177237634 * 100 / 2717440206 21:31
camelia 6.52222755845
timo the total Ir count as reported by callgrind; this is the sum of three runs of rakudo -e '' 21:32
for the entire validate_static_frame function
21:41 sena_kun left
Geth MoarVM/optimize_hllbool_boolify_pairs: 14a8befd1c | (Stefan Seifert)++ | 3 files
Eliminate hllbool/boot-boolify-boxed-int pairs in spesh

No need to turn an int into an HLL bool just to turn it back to an int when using it as a condition for jumps. Eliminate those pairs same as we do with box/unbox.
21:42 sena_kun joined 22:04 dogbert17 joined 22:06 dogbert11 left 22:12 RakuIRCLogger left 22:13 RakuIRCLogger joined