[00:08] *** reportable6 left [00:09] *** reportable6 joined [00:12] ¦ MoarVM: MasterDuke17++ created pull request #1682: Add missing _u cases to disp program callsite code [00:12] ¦ MoarVM: review: https://github.com/MoarVM/MoarVM/pull/1682 [00:13] vrurg: ^^^ [00:13] off to sleep [00:33] MasterDuke: would wait until the next bump, perhaps. [02:24] *** harrow left [02:30] *** harrow joined [03:41] *** evalable6 left [03:41] *** linkable6 left [03:42] *** evalable6 joined [04:42] *** bloatable6 left [04:42] *** benchable6 left [04:42] *** squashable6 left [04:42] *** evalable6 left [04:42] *** coverable6 left [04:42] *** nativecallable6 left [04:42] *** sourceable6 left [04:42] *** statisfiable6 left [04:42] *** committable6 left [04:42] *** unicodable6 left [04:42] *** shareable6 left [04:42] *** notable6 left [04:42] *** bisectable6 left [04:42] *** releasable6 left [04:42] *** tellable6 left [04:42] *** reportable6 left [04:43] *** releasable6 joined [04:44] *** bisectable6 joined [04:44] *** reportable6 joined [04:44] *** squashable6 joined [04:44] *** unicodable6 joined [05:43] *** tellable6 joined [05:43] *** notable6 joined [05:44] *** linkable6 joined [05:44] *** sourceable6 joined [05:44] *** shareable6 joined [05:44] *** committable6 joined [05:45] *** benchable6 joined [06:43] *** evalable6 joined [06:43] *** nativecallable6 joined [06:44] *** bloatable6 joined [06:45] *** statisfiable6 joined [07:45] *** coverable6 joined [07:47] ¦ MoarVM: b0723bcb29 | (Daniel Green)++ | src/spesh/disp.c [07:47] ¦ MoarVM: Add missing _u cases to disp program callsite code [07:47] ¦ MoarVM: [07:47] ¦ MoarVM: Fixes the `MoarVM panic: Unknown dispatch op when resolving callsite` in [07:47] ¦ MoarVM: HTTP::HPACK. [07:47] ¦ MoarVM: review: https://github.com/MoarVM/MoarVM/commit/b0723bcb29 [07:47] ¦ MoarVM: a1a45e57ed | niner++ (committed using GitHub Web editor) | src/spesh/disp.c [07:47] ¦ MoarVM: Merge pull request #1682 from MasterDuke17/add_some__u_cases_to_disp_program_callsite_code [07:47] ¦ MoarVM: [07:47] ¦ MoarVM: Add missing _u cases to disp program callsite code [07:47] ¦ MoarVM: review: https://github.com/MoarVM/MoarVM/commit/a1a45e57ed [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 [09:39] *** discord-raku-bot joined [10:09] moarning o/ [10:10] * 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:11] jnthnwrthngtn: sounds like you really need a cup of coffe right now [10:13] Ah the dreaded coffee bootstrap [10:22] *** Geth left [10:22] *** RakuIRCLogger__ joined [10:22] *** TempIRCLogger left [10:22] *** Geth joined [10:23] *** lizmat left [10:23] *** RakuIRCLogger left [10:23] *** lizmat joined [10:24] *** TempIRCLogger joined [10:27] *** RakuIRCLogger__ left [10:27] *** RakuIRCLogger joined [10:31] \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 [10:32] flood and panic. She felt bad. I said I'd goofed this sort of thing before, but got it right at this hotel. [10:38] huh. i was looking at https://en.algorithmica.org/hpc/algorithms/gcd/ and https://github.com/lemire/Code-used-on-Daniel-Lemire-s-blog/blob/master/2013/12/26/gcd.cpp and if i add sergey's version to lemir's benchmark and run it, the sort of naive https://github.com/lemire/Code-used-on-Daniel-Lemire-s-blog/blob/master/2013/12/26/gcd.cpp#L38-L64 is [10:38] noticeably fastest [10:38] but it's quite a bit slower when put in moarvm [11:00] Different optimization flags, maybe? [11:00] the flags are pretty similar. might be size of variables, trying to make them consistent with what moarvm uses now [11:01] will double check flags next [11:03] hm, it is c++ vs c. but there aren't very many c++ features being used... [11:08] iirc there are a couple of things that are ub in c++ but not in c [11:09] unbounded recursion, and some usage of unions ... probably doesn't apply here [11:18] 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:39] 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! [12:02] 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:09] 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:27] *** evalable6 left [12:27] *** 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 [15:27] isn't the ternary unnecessary here? https://github.com/MoarVM/MoarVM/blob/master/src/6model/bootstrap.c#L374 [15:30] because the result should be statically initialized to 0 when created, right? https://github.com/MoarVM/MoarVM/blob/master/src/core/args.c#L600 [15:34] 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 https://github.com/MoarVM/MoarVM/blob/master/src/core/args.c#L567 [15:35] huh, it's zero after just the declaration at '-O3', but not at '-O0' [15:41] 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:42] am i missing anything? [15:47] 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:49] *** evalable6 left [15:49] *** linkable6 left [15:49] *** evalable6 joined [15:51] *** linkable6 joined [16:00] Stack variables are uninitialized by default in C. If they happen to be 0 that's just a coincidence. [16:03] ok, but setting the union to NULL will always zero them, correct? [16:05] exists is not in a union [16:06] yep, but here `result.arg.s = NULL;` arg is the union [16:07] But how does that help us avoid returning a struct? [16:11] 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:21] hm, https://github.com/MoarVM/MoarVM/blob/master/src/core/interp.c#L1090-L1096 is going to be a problem [16:23] 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 [16:33] 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:34] but it's the optional ones, both positional and named that are trickier [16:37] how much benefit is there to jitting these param ops? [16:44] oh, looks like spesh can change them into sp_getarg_* (so no benefit to jitting when that happens) [17:01] Yes, argument speshing is way better than just JITing. [17:02] yeah, maybe this is another experiment that's not really going to be worth the time [17:04] however, i'm seeing a lego jit bail in find_method because of param_on_o [17:05] this one https://github.com/rakudo/rakudo/blob/master/src/Perl6/Metamodel/MROBasedMethodDispatch.nqp#L8-L35 [17:05] and i'm not sure how bad that is [17:09] The first question is always: "why does arg spesh fail?". The answer is in the spesh log [17:10]       # [000] bailed argument spesh: unhandled coercion [17:10]       param_on_o        r3(2), lits(no_fallback),   BB(3) [17:25] Oh....but which case? [17:27] In any case it seems to lack support for param_rn_u and param_on_u [17:27] jnthnwrthngtn: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000216.html [17:32] 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:33] Worth making an issue there to request the Windows DLL is updated. [17:33] jnthnwrthngtn: I guess... but maybe an update to the OpenSSL modules could ensure an up-to-date OpenSSL is used ? [17:34] 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:35] Ensuring that such libraries are up to date is the operating system's job [17:35] nine: but we could help with that, no? [17:35] How? [17:36] 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 ? [17:38] 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:41] ok.... :-) [17:43] 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. [18:02] understood :-) [18:19] *** discord-raku-bot left [19:07] *** ShaneC joined [19:15] *** [Coke] left [19:17] *** [Coke] joined [19:31] *** discord-raku-bot joined [19:47] nine: it's this case https://github.com/MoarVM/MoarVM/blob/master/src/spesh/args.c#L654-L660 and found_flag is MVM_CALLSITE_ARG_INT [20:13] MasterDuke: sounds like a native int is passed in $no_fallback when we expect an object [20:14] yeah, looks like this fixes it: [20:14]              case MVM_OP_param_on_o: [20:14] -                if (found_idx != -1 && !(found_flag & MVM_CALLSITE_ARG_OBJ)) { [20:14] +                if (found_idx != -1 && !(found_flag & MVM_CALLSITE_ARG_OBJ) [20:14] +                        && !(prim_spec(tc, type_tuple, found_flag_idx) == MVM_STORAGE_SPEC_BP_NONE)) { [20:17] Wait...are you saying that due to a bug we never actually entered this case? https://github.com/MoarVM/MoarVM/blob/master/src/spesh/args.c#L1033 [20:18] don't know [20:24] It looks like that. Does it pass a spectest with blocking and nodelay? [20:24] passes a spectest, didn't try with blocking and nodelay [20:24] blocking and nodelay are always a good idea when working on spesh [20:25] that case does get hit a couple times, but always ends up in the found_idx == 1 branch [20:25] *-1 [20:27] Not very surprising. Optional names are probably omitted a lot [20:28] *** ShaneC left [20:28] *** ShaneC joined [20:29] fyi, i had my prim_spec patch live when i saw that case hit, haven't tried yet without (currently running another spectest) [20:30] and oh wow, it's slow. m-test took 99s instead of 22s [20:45] 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; [20:45] rakudo-moar 3bde138cb: OUTPUT: «1.770201144␤10000001␤» [20:46] locally that drops from ~1.66s to 1.55s with MVM_SPESH_BLOCKING=1 [20:56] very nice :) [20:58] ¦ MoarVM: MasterDuke17++ created pull request #1683: Fix speshing of param_on_o when we can coerce the arg [20:58] ¦ MoarVM: review: https://github.com/MoarVM/MoarVM/pull/1683 [21:07] 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:08] which case? [21:09] maybe https://github.com/MoarVM/MoarVM/blame/master/src/spesh/args.c#L654-L660 [21:09] s/maybe// [21:10] maybe it'd be better to just remove it entirely? or leave it in with just a comment and a break? [21:17] Ooooh...that explains it! Well yes, then that case is superfluous [23:12] ¦ MoarVM: 0b45dd48bb | (Daniel Green)++ | src/spesh/args.c [23:12] ¦ MoarVM: Fix speshing of param_on_o [23:12] ¦ MoarVM: [23:12] ¦ MoarVM: This case was added in https://github.com/MoarVM/MoarVM/pull/1681, but [23:12] ¦ MoarVM: isn't necessary and causes spesh to not turn it into an sp_* op, and then [23:12] ¦ MoarVM: since it can't be jitted either it was causing a bail. So remove the [23:12] ¦ MoarVM: body of the case, but leave it in so nobody else thinks it was [23:12] ¦ MoarVM: accidentally left out. [23:12] ¦ MoarVM: review: https://github.com/MoarVM/MoarVM/commit/0b45dd48bb [23:12] ¦ MoarVM: 588e00d60e | MasterDuke17++ (committed using GitHub Web editor) | src/spesh/args.c [23:12] ¦ MoarVM: Merge pull request #1683 from MasterDuke17/correctly_spesh_param_on_o [23:12] ¦ MoarVM: review: https://github.com/MoarVM/MoarVM/commit/588e00d60e [23:41] *** leont left [23:42] *** leont joined [23:43] *** AlexDaniel left [23:56] *** AlexDaniel joined