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:08
reportable6 left
00:09
reportable6 joined
|
|||
Geth | MoarVM: MasterDuke17++ created pull request #1682: Add missing _u cases to disp program callsite code |
00:12 | |
MasterDuke | vrurg: ^^^ | 00:13 | |
off to sleep | |||
vrurg | MasterDuke: would wait until the next bump, perhaps. | 00:33 | |
02:24
harrow left
02:30
harrow joined
03:41
evalable6 left,
linkable6 left
03:42
evalable6 joined
04:42
bloatable6 left,
benchable6 left,
squashable6 left,
evalable6 left,
coverable6 left,
nativecallable6 left,
sourceable6 left,
statisfiable6 left,
committable6 left,
unicodable6 left,
shareable6 left,
notable6 left,
bisectable6 left,
releasable6 left,
tellable6 left,
reportable6 left
04:43
releasable6 joined
04:44
bisectable6 joined,
reportable6 joined,
squashable6 joined,
unicodable6 joined
05:43
tellable6 joined,
notable6 joined
05:44
linkable6 joined,
sourceable6 joined,
shareable6 joined,
committable6 joined
05:45
benchable6 joined
06:43
evalable6 joined,
nativecallable6 joined
06:44
bloatable6 joined
06:45
statisfiable6 joined
07:45
coverable6 joined
|
|||
Geth | MoarVM: b0723bcb29 | (Daniel Green)++ | src/spesh/disp.c Add missing _u cases to disp program callsite code Fixes the `MoarVM panic: Unknown dispatch op when resolving callsite` in HTTP::HPACK. |
07:47 | |
MoarVM: a1a45e57ed | niner++ (committed using GitHub Web editor) | src/spesh/disp.c Merge pull request #1682 from MasterDuke17/add_some__u_cases_to_disp_program_callsite_code Add missing _u cases to disp program callsite code |
|||
08:11
Shane__ left
08:21
frost joined
09:20
discord-raku-bot left
09:31
discord-raku-bot joined
09:39
discord-raku-bot left,
discord-raku-bot joined
|
|||
jnthnwrthngtn | moarning o/ | 10:09 | |
jnthnwrthngtn just spent a minute looking for his coffee cup...before realizing it was on his desk, full of coffee, made 2 minutes beforehand... | 10:10 | ||
gfldex | jnthnwrthngtn: sounds like you really need a cup of coffe right now | 10:11 | |
nine | Ah the dreaded coffee bootstrap | 10:13 | |
10:22
Geth left,
RakuIRCLogger__ joined,
TempIRCLogger left,
Geth joined
10:23
lizmat left,
RakuIRCLogger left,
lizmat joined
10:24
TempIRCLogger joined
10:27
RakuIRCLogger__ left,
RakuIRCLogger joined
|
|||
Nicholas | \o | 10:31 | |
today at the hotel breakfast some poor lady had a coffee bootstrapping problem - before first coffee, didn't correctly work out which sized cup to put under the coffee machine | |||
flood and panic. She felt bad. I said I'd goofed this sort of thing before, but got it right at this hotel. | 10:32 | ||
MasterDuke | huh. i was looking at en.algorithmica.org/hpc/algorithms/gcd/ and github.com/lemire/Code-used-on-Dan...26/gcd.cpp and if i add sergey's version to lemir's benchmark and run it, the sort of naive github.com/lemire/Code-used-on-Dan...pp#L38-L64 is | 10:38 | |
noticeably fastest | |||
but it's quite a bit slower when put in moarvm | |||
jnthnwrthngtn | Different optimization flags, maybe? | 11:00 | |
MasterDuke | the flags are pretty similar. might be size of variables, trying to make them consistent with what moarvm uses now | ||
will double check flags next | 11:01 | ||
hm, it is c++ vs c. but there aren't very many c++ features being used... | 11:03 | ||
moon-child | iirc there are a couple of things that are ub in c++ but not in c | 11:08 | |
unbounded recursion, and some usage of unions ... probably doesn't apply here | 11:09 | ||
MasterDuke | weird. changed all the sizes, timings in the benchmark stayed the same. put a different one from the benchmark that was also faster, but in moarvm it was slower | 11:18 | |
i even tried putting in one version that the benchmark said was slower, in case it was actually backwards. but nope, slower than stock in moarvm also! | 11:39 | ||
hm, it's very cpu architecture dependent. on my haswell laptop, sergey's version is much faster, ~2.5s instead of ~4.6s for master. on my zen2 desktop, sergey's version is ~2.1s instead of ~2.0s for master. both using gcc 11.2.0 | 12:02 | ||
ok, this isn't usually hot code so i'm not going to bother trying to put in a tuning step during moarvm building to choose different versions. experiment finished | 12:09 | ||
12:27
evalable6 left,
linkable6 left
12:56
Kaipi left
12:59
Kaipi joined
13:08
frost left
13:27
evalable6 joined
13:30
linkable6 joined
13:46
frost joined
14:07
frost left
|
|||
MasterDuke | isn't the ternary unnecessary here? github.com/MoarVM/MoarVM/blob/mast...rap.c#L374 | 15:27 | |
because the result should be statically initialized to 0 when created, right? github.com/MoarVM/MoarVM/blob/mast...rgs.c#L600 | 15:30 | ||
oh hm, according to a quick test on godbolt, just declaring one won't initialize it to 0, but setting the MVMString * part of the union to NULL will github.com/MoarVM/MoarVM/blob/mast...rgs.c#L567 | 15:34 | ||
huh, it's zero after just the declaration at '-O3', but not at '-O0' | 15:35 | ||
which means we can change MVM_args_get_named_int to actually return an int, not the struct, and then param_rn_i turns into just a function call and can be easily jitted | 15:41 | ||
am i missing anything? | 15:42 | ||
and for the str and obj version, it looks like everywhere .exists on the result is checked, the value would be NULL when !.exists and not-NULL when .exists, so we can instead just check if the value is NULL | 15:47 | ||
15:49
evalable6 left,
linkable6 left,
evalable6 joined
15:51
linkable6 joined
|
|||
nine | Stack variables are uninitialized by default in C. If they happen to be 0 that's just a coincidence. | 16:00 | |
MasterDuke | ok, but setting the union to NULL will always zero them, correct? | 16:03 | |
nine | exists is not in a union | 16:05 | |
MasterDuke | yep, but here `result.arg.s = NULL;` arg is the union | 16:06 | |
nine | But how does that help us avoid returning a struct? | 16:07 | |
MasterDuke | because the default value that's set everywhere that .exists is checked is 0, or something not-NULL. i think we still need .exists for other things, but for setting a default value we can check against the value being NULL for the objects and just pass through the value for the primitives | 16:11 | |
hm, github.com/MoarVM/MoarVM/blob/mast...1090-L1096 is going to be a problem | 16:21 | ||
well, the positionals have both `*_required_*` and `*_optional_*` version, where the required do return the type they're looking for and the optional return an ArgInfo | 16:23 | ||
guess that's what would actually have to be done | |||
ah, so currently we don't jit any optional param ops or named param ops, just required positional. adding required named wouldn't be too bad (just splitting the current MVM_args_get_named_*` into required and optional versions | 16:33 | ||
but it's the optional ones, both positional and named that are trickier | 16:34 | ||
how much benefit is there to jitting these param ops? | 16:37 | ||
oh, looks like spesh can change them into sp_getarg_* (so no benefit to jitting when that happens) | 16:44 | ||
nine | Yes, argument speshing is way better than just JITing. | 17:01 | |
MasterDuke | yeah, maybe this is another experiment that's not really going to be worth the time | 17:02 | |
however, i'm seeing a lego jit bail in find_method because of param_on_o | 17:04 | ||
this one github.com/rakudo/rakudo/blob/mast...nqp#L8-L35 | 17:05 | ||
and i'm not sure how bad that is | |||
nine | The first question is always: "why does arg spesh fail?". The answer is in the spesh log | 17:09 | |
MasterDuke | Ā Ā Ā Ā Ā # [000] bailed argument spesh: unhandled coercion | 17:10 | |
Ā Ā Ā Ā Ā param_on_oĀ Ā Ā Ā Ā Ā Ā r3(2), lits(no_fallback),Ā Ā BB(3) | |||
nine | Oh....but which case? | 17:25 | |
In any case it seems to lack support for param_rn_u and param_on_u | 17:27 | ||
lizmat | jnthnwrthngtn: mta.openssl.org/pipermail/openssl-...00216.html | ||
jnthnwrthngtn | lizmat: I figure since it's an update to a shared library most folks will get such updates via their OS package managers; the exception is Windows where the OpenSSL module bundles a DLL, but I'm not the maintainer of that module. | 17:32 | |
Worth making an issue there to request the Windows DLL is updated. | 17:33 | ||
lizmat | jnthnwrthngtn: I guess... but maybe an update to the OpenSSL modules could ensure an up-to-date OpenSSL is used ? | ||
jnthnwrthngtn | If there's a new release of the Raku OpenSSL module with said DLL I can ship an IO::Socket::Async::SSL that requires a minimum version. I don't believe there's much the OpenSSL module itself can do otherwise since it's a native dependency? | 17:34 | |
nine | Ensuring that such libraries are up to date is the operating system's job | 17:35 | |
lizmat | nine: but we could help with that, no? | ||
jnthnwrthngtn | How? | ||
lizmat | I assumed there was a way to interrogate the library for its version | 17:36 | |
if the version is too low, decided on some action ? | |||
nine | Why should we? OpenSSL is used by hundreds of software packages. I would positively hate them all to tell me about some OpenSSL update | 17:38 | |
lizmat | ok.... :-) | 17:41 | |
nine | To expand on that: OpenSSL is managed by my operating system. I fully expect to get the security update installed automatically in the next few days. There's nothing I have to do and until the update is available, there's nothing I can do. It would not help me and only harm me if I e.g. tried to install Cro and can't because the OpenSSL Raku module is refusing to run. | 17:43 | |
lizmat | understood :-) | 18:02 | |
18:19
discord-raku-bot left
19:07
ShaneC joined
19:15
[Coke] left
19:17
[Coke] joined
19:31
discord-raku-bot joined
|
|||
MasterDuke | nine: it's this case github.com/MoarVM/MoarVM/blob/mast...#L654-L660 and found_flag is MVM_CALLSITE_ARG_INT | 19:47 | |
nine | MasterDuke: sounds like a native int is passed in $no_fallback when we expect an object | 20:13 | |
MasterDuke | yeah, looks like this fixes it: | 20:14 | |
Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā case MVM_OP_param_on_o: | |||
-Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā if (found_idx != -1 && !(found_flag & MVM_CALLSITE_ARG_OBJ)) { | |||
+Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā if (found_idx != -1 && !(found_flag & MVM_CALLSITE_ARG_OBJ) | |||
+Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā && !(prim_spec(tc, type_tuple, found_flag_idx) == MVM_STORAGE_SPEC_BP_NONE)) { | |||
nine | Wait...are you saying that due to a bug we never actually entered this case? github.com/MoarVM/MoarVM/blob/mast...gs.c#L1033 | 20:17 | |
MasterDuke | don't know | 20:18 | |
nine | It looks like that. Does it pass a spectest with blocking and nodelay? | 20:24 | |
MasterDuke | passes a spectest, didn't try with blocking and nodelay | ||
nine | blocking and nodelay are always a good idea when working on spesh | ||
MasterDuke | that case does get hit a couple times, but always ends up in the found_idx == 1 branch | 20:25 | |
*-1 | |||
nine | Not very surprising. Optional names are probably omitted a lot | 20:27 | |
20:28
ShaneC left,
ShaneC joined
|
|||
MasterDuke | fyi, i had my prim_spec patch live when i saw that case hit, haven't tried yet without (currently running another spectest) | 20:29 | |
and oh wow, it's slow. m-test took 99s instead of 22s | 20:30 | ||
passed | 20:45 | ||
m: sub a(:$a) { $a ?? $a + 1 !! 0 }; my int $a = 1; for ^10_000_000 { $a = a(:$a); }; say now - INIT now; say $a; | |||
camelia | 1.770201144 10000001 |
||
MasterDuke | locally that drops from ~1.66s to 1.55s with MVM_SPESH_BLOCKING=1 | 20:46 | |
nine | very nice :) | 20:56 | |
Geth | MoarVM: MasterDuke17++ created pull request #1683: Fix speshing of param_on_o when we can coerce the arg |
20:58 | |
MasterDuke | hm. i actually added that case in my recent PR because i noticed it was missing. but i guess the omission was intentional, because everything should be able to coerce to obj, right? | 21:07 | |
nine | which case? | 21:08 | |
MasterDuke | maybe github.com/MoarVM/MoarVM/blame/mas...#L654-L660 | 21:09 | |
s/maybe// | |||
maybe it'd be better to just remove it entirely? or leave it in with just a comment and a break? | 21:10 | ||
nine | Ooooh...that explains it! Well yes, then that case is superfluous | 21:17 | |
Geth | MoarVM: 0b45dd48bb | (Daniel Green)++ | src/spesh/args.c Fix speshing of param_on_o This case was added in github.com/MoarVM/MoarVM/pull/1681, but isn't necessary and causes spesh to not turn it into an sp_* op, and then since it can't be jitted either it was causing a bail. So remove the body of the case, but leave it in so nobody else thinks it was accidentally left out. |
23:12 | |
MoarVM: 588e00d60e | MasterDuke17++ (committed using GitHub Web editor) | src/spesh/args.c Merge pull request #1683 from MasterDuke17/correctly_spesh_param_on_o |
|||
23:41
leont left
23:42
leont joined
23:43
AlexDaniel left
23:56
AlexDaniel joined
|