github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm Set by AlexDaniel on 12 June 2018. |
|||
00:24
dogbert11 joined
01:02
dogbert17 joined
01:04
dogbert11 left
01:14
dogbert2 joined
01:15
dogbert11 joined
01:17
dogbert17 left
01:18
dogbert2 left
02:04
dogbert17 joined
02:07
dogbert2 joined,
dogbert11 left
02:10
dogbert11 joined
02:11
dogbert12 joined,
dogbert17 left,
dogbert2 left
02:15
dogbert11 left
04:37
coverable6 left,
sourceable6 left
04:40
tbrowder left
04:41
tbrowder joined
04:42
coverable6 joined,
sourceable6 joined,
statisfiable6 joined,
shareable6 joined,
tellable6 joined,
unicodable6 joined,
bloatable6 joined,
squashable6 joined,
notable6 joined,
greppable6 joined,
bisectable6 joined,
cog_ joined
|
|||
nwc10 | good *, #moarvm | 07:03 | |
(and immediately AFK to get coffee) | |||
07:15
sortiz joined
08:08
domidumont joined
08:09
MasterDuke joined
|
|||
MasterDuke | jnthn: not sure exactly what you mean re memory vs speed? we could spesh more of the callsites instead of creating templates? on average i'm likely to choose speed over memory use, especially since these came up with regex/grammar related stuff, which i'd love to get massively faster | 08:17 | |
sortiz | MasterDuke: I think that I found the problem in github.com/MoarVM/MoarVM/issues/1432, a proposed path in my comment. | 08:25 | |
s/path/patch/ | 08:26 | ||
MasterDuke | sortiz: so `d_repr_data` is guaranteed never to be NULL after your patch? in that case i'd change its declaration to not have the ternary that looks like it could be so | 08:32 | |
nine | MasterDuke: it was never NULL even before the patch | 08:34 | |
MasterDuke | and it could be removed from the if here github.com/MoarVM/MoarVM/blob/mast...ray.c#L822 | ||
well, yeah, s_repr_data was. but could d_repr_data ever be? | 08:35 | ||
nine | no | ||
MasterDuke | well then i think its declaration and that if should be changed | 08:37 | |
sortiz | Before the patch, src was erroneously used in line 815, so d_repr_data was NULL also. | 08:41 | |
08:52
Kaeipi joined
08:54
Kaeipi left
09:03
Kaiepi joined
09:14
sortiz left
09:21
zakharyas joined
09:26
MasterDuke left
10:03
cog__ joined
10:06
cog_ left
10:28
patrickb joined
|
|||
nine | MasterDuke: one thing I noticed when looking at remove_spesh_optimization_if_it_has_too_many_deopts: the "Invalid owner in item added to GC worklist" errors are always about frame registers. | 10:50 | |
tellable6 | nine, I'll pass your message to MasterDuke | ||
El_Che | hi, moarvm can depend on libzstd-dev. Most distros besides rhel 7, do have it. Is there a big advantage depending on that? If not. I'll rather keep the rakudo pkgs dependency free | 11:40 | |
jnthn | iirc, it's used for heap snapshots, and without it you get them in an old/deprecated format | 11:42 | |
nine | Isn't taking care of dependencies exactly what distros are for? | ||
El_Che | nine: that isn't the question :_ | 11:43 | |
:) | |||
Adding a dependency is OK, just making sure it's worth it. From jnthn's response I gather it is | 11:44 | ||
jnthn | I was going to look into seeing if we can provide that library in source builds also (as we do with other deps), because it's going to be a bit disappointing if we get heap snapshot visualization stuff in Comma and then none of the standard builds / packages support it. | ||
(Because MoarVM was built without the dep) | |||
El_Che | for now only ubi:7 (rhel) doesn't seem to have the package | ||
(centos 7 does) | |||
jnthn | Ubuntu 16.04 doesn't have it, iirc | 11:45 | |
Oh wait, maybe it does | |||
I guess that's out of support already too, or going that way soon :) | |||
jnthn will finally use something more modern soon, because new hardware => gotta install something anyway :) | 11:46 | ||
El_Che | jnthn: it has it, I added to the build process but not yet as a dependency | ||
jnthn | OK. | ||
El_Che | the new pkg hoster only has "el" for CentOS/RHEL/Amazon so I can not upload both Centos 7 (with the lib) and rhel 7 (without) packages | 11:48 | |
nine | The openSUSE installation on my desktop is about 20 years old now... got files from 2001 in my /etc and /opt directories | 11:49 | |
El_Che | rhel 8 has the package | ||
jnthn | Lunch time, then some more new-disp hacking this afternoon :) | 11:52 | |
12:03
MasterDuke joined
|
|||
MasterDuke | . | 12:03 | |
tellable6 | 2021-02-10T10:50:19Z #moarvm <nine> MasterDuke: one thing I noticed when looking at remove_spesh_optimization_if_it_has_too_many_deopts: the "Invalid owner in item added to GC worklist" errors are always about frame registers. | ||
MasterDuke | huh, hadn't noticed that | 12:04 | |
12:10
zakharyas left
|
|||
nine | Could be coincidence, but I somewhat doubt it. Fits too well with that the branch is about spesh candidates, i.e. frames | 12:10 | |
MasterDuke | i'll look through frame.c again, see if i spot anything missing | 12:11 | |
well this is interesting. i just noticed github.com/MoarVM/MoarVM/blob/mast...frame.c#L7 for the first time so turned it on. now i get a segv almost right away in the nqp build (with it off i get a ways into the nqp build before the panic) | 12:18 | ||
nine | MasterDuke: that seems to throw errors in a normal build of another branch as well | 12:21 | |
MasterDuke | i changed code inside some `#if MVM_SPESH_CHECK_PRESELECTION`, but never compiled it until now | 12:22 | |
might have been in the previous PR. is your branch after that was merged? | |||
oh, there's only that one block | 12:24 | ||
huh. well i added an `if (cands_and_arg_guards)` to that block, and with MVM_SPESH_CHECK_PRESELECTION turned on i get a ton of output like `Inconsistent spesh preselection of 'as_mast' (111): got 7, not 5` | 12:36 | ||
ah, get a ton of them on master as well | 12:38 | ||
and even if i back up to before my convert_MVMSpeshCandidate_to_ REPR PR | 12:41 | ||
is that a problem? | |||
nine | Could be just a "this warrants a look but doesn't necessarily have to be bad" and therefore OK. Or could be the source of those random segfaults we see... Who knows? Oh, jnthn would :) | 12:44 | |
MasterDuke looks for that butterfly-shaped spotlight | |||
12:46
Kaiepi left,
Kaiepi joined
12:47
Kaiepi left
12:48
Kaiepi joined
|
|||
jnthn | I doubt that'd lead to segfaults, though it is a bit odd | 12:54 | |
Oh...hmm | |||
I do wonder if removing specializations could break spesh linking though | 12:55 | ||
In that we identify which specialized candidate to invoke from another one | |||
And if the indices change I guess that may be bad | |||
MasterDuke | hm, where does that linking happen> | 12:56 | |
? | |||
jnthn | github.com/MoarVM/MoarVM/blob/mast...ze.c#L2110 | 12:57 | |
MasterDuke | ugh | 12:58 | |
jnthn | I'd forgotten this used indices. That's the only way it could have worked before I guess | 12:59 | |
That may well be the problem, anyway | 13:00 | ||
13:00
sneakyness joined,
sneakyness left
|
|||
jnthn | The next question is what to do about it. | 13:00 | |
MasterDuke | yeah... | 13:01 | |
jnthn | We probably need this candidate to end up discarded also | ||
(that is, the caller linked to it) | |||
Avoiding the index could be done by sticking the MVMSpeshCandidate we want to call into a spesh slot and then having the index of that be used in the op | 13:02 | ||
iirc the candidate caries a "I've been invalidated" flag; we could then not use it, and maybe log a deopt on the caller, so that it'll eventually accumulate enough points to be worth throwing out also | 13:03 | ||
Or we can signal explicitly when we log the fake-deopt of it that it's linked to a dead optimized version so should be more expediently dropped too | 13:04 | ||
These two could be done separately; the first part is just correctness, the second part is performance | 13:05 | ||
13:08
patateTX joined
|
|||
patateTX | /!\ this channel has moved to ##hamradio /!\ | 13:08 | |
13:08
MasterDuke left,
MasterDuke joined,
patateTX left
|
|||
MasterDuke | so instead of `new_operands[1].lit_i16 = spesh_cand;` have `new_operands[1].lit_i16 = MVM_spesh_add_spesh_slot_try_reuse(tc, g, target_sf->body.spesh->body.spesh_candidates[spesh_cand]);`? | 13:10 | |
nine | like that | ||
13:11
AzureeO joined
|
|||
jnthn | Yes, then you'll need to also update sp_invoke_* in interp.c and their JIT | 13:11 | |
AzureeO | /!\ this channel has moved to #nyymit /!\ | ||
13:12
AzureeO left
|
|||
jnthn | ...that's an odd new kind of spam | 13:12 | |
13:13
bigpreshZx joined
|
|||
bigpreshZx | /!\ this channel has moved to #nyymit /!\ | 13:13 | |
lizmat | yeah, looks like it is testing the waters ? | ||
MasterDuke | cool, i'll give that a go | ||
13:14
bigpreshZx left,
Guest29713 joined
|
|||
Guest29713 | /!\ this channel has moved to #nyymit /!\ | 13:15 | |
13:15
vdamewoodTX joined
|
|||
vdamewoodTX | /!\ this channel has moved to #nyymit /!\ | 13:15 | |
13:15
vdamewoodTX left,
Guest29713 left
|
|||
lizmat just pushed some commits making the Spesh Log report also include slow specializations | 13:21 | ||
gist.github.com/lizmat/654d3658482...c104cca99b | |||
hope this can be useful for folks around here... and if there are other parts of a spesh log you'd like to see reported like that, please let me know | |||
El_Che | what is the "better" REPL helper nowadays? | 13:30 | |
both options seem unmaintained | 13:32 | ||
patrickb | What are the options? | 13:33 | |
El_Che | Linenoise and late drforr's perl5-readline | 13:34 | |
MasterDuke | lizmat: "Specializations that did *NOT* get JITted (times in us)" shouldn't that be "Routines that..."? | 13:35 | |
lizmat | I think technically they're frames ? | 13:36 | |
El_Che | readline is not part of the community modules so it assume it's abandoned after drforr passed | ||
lizmat | I thought someone else had taken over Readline ? | ||
fooist ? | |||
El_Che | github.com/fooist/perl6-readline | 13:38 | |
lizmat | afk& | 13:40 | |
MasterDuke | hm, MVM_frame_invoke is going to have to change to take the actual MVMSpeshCandidate instead of an index too, correct? | 13:49 | |
jnthn | Yes | 13:53 | |
MasterDuke | k | ||
14:10
MasterDuke left
14:11
MasterDuke joined
14:27
lucasb joined
|
|||
nine | Well to me the name spesh_cand already sounded like it'd be a pointer. Nice of you to change reality to match my expectations :D | 14:35 | |
jnthn | .oO( I knew it was coming :P ) |
||
14:38
domidumont left
|
|||
MasterDuke waits for the top to wobble | 15:10 | ||
jnthn | The nested resumptions is a bit tricky. | 15:12 | |
Hmm. | 15:13 | ||
moritz | I don't really know what you're talking about, but "The nested resumptions is a bit tricky." sounds like quite an understatement :-) | 15:16 | |
MasterDuke | at first i thought it was a further reference to Inception, but maybe it's about new-disp | 15:17 | |
jnthn | It's handling the situation where, for example, we make a method call, and that method is wrapped, so callsame is initially about the wrappers, but at some point those are exhausted and we want to go on with the methods bit | 15:21 | |
Which is the situation that fired the starting gun on new-disp anyway :) | |||
It's the first thing that causes me to care a lot about order of code-gen in dispatch programs | 15:22 | ||
Which I'd not quite realized would happen | |||
16:05
Kaiepi left
16:06
Kaiepi joined
16:13
Kaiepi left
16:14
Kaiepi joined
16:46
domidumont joined
|
|||
jnthn | Mostly figured it out, in theory... And down to one screenful of compile errors in the dispatch program production. Need a break... | 17:17 | |
17:44
MasterDuke left
18:26
MasterDuke joined
18:29
domidumont left
19:06
MasterDuke left
19:26
lucasb left
19:28
Kaiepi left
19:29
Kaiepi joined
20:10
squashable6 left
20:13
squashable6 joined
20:24
patrickb left
20:37
MasterDuke joined
|
|||
MasterDuke | well hello there, nqp just built successfully | 20:41 | |
and rakudo | 20:46 | ||
jnthn | Ooh, after the spesh linking fix? | 20:49 | |
20:53
MasterDuke left
|
|||
nine | what a way to keep us in suspense | 21:06 | |
jnthn | I was doing grocery ordering all this time anyway :) | 21:16 | |
21:24
zakharyas joined
21:28
squashable6 left
21:32
squashable6 joined
21:38
zakharyas left
22:20
MasterDuke joined
|
|||
MasterDuke | heh, yeah (mostly) | 22:22 | |
i stuck the candidate in a spesh slot and made the change to MVM_frame_invoke | 22:23 | ||
however, i have not yet invalidated the caller or anything like that | |||
i am a little unclear exactly what you meant | 22:24 | ||
or made the jit changes yet (i've been building/running with the jit disabled this whole time) | 22:26 | ||
but hey, nqp tests and a spectest all pass | 22:33 | ||
jnthn++ i don't think i ever would have found that spesh linking problem | 22:35 | ||
jnthn | I only wish I'd thought of it sooner, so you didn't spend so much time trying to figure it ut | 23:11 | |
*out | |||
MasterDuke | btw, this is interesting. i forgot to run with MVM_JIT_DISABLE=1 when building rakudo and running a spectest, but everything was fine, even though i haven't touched any of the jit code | 23:12 | |
jnthn | I dunno if FSA debug + Valgrind/ASAN were employed together to hunt this, but you were also a bit unlucky if they were, because I'd have expected an out of bounds somewhere. | ||
And it woulda probably pointed somewhere where it'd have been obvious to me what was going on | 23:13 | ||
MasterDuke | yeah, i've had FSA debug turned on, building with --valgrind, and tried --asan a couple times, but they never revealed anything extra beyond that panic (or the segv in the hash code if GC_DEBUG wasn't 3) | ||
jnthn | Odd. I'd have expected we'd have tried to read beyond the end of the candidates array. | 23:14 | |
Not always, but at least somewhere | |||
MasterDuke | maybe it would have during the rakudo build, lots more removing opts there | 23:15 | |
only a couple when building nqp before the segv/panic | |||
guess i could have tried decreasing the number of deopts required for removing an opt | |||
oh well | 23:16 | ||
so is this github.com/MoarVM/MoarVM/blob/mast...dasc#L3377 not getting hit? because MVM_frame_invoke_code now expects an MVMSpeshCandidate as the last arg, not an MVMuint32 | 23:18 | ||
jnthn | I'd be a bit surprised if it's not being | 23:19 | |
oh, how do you find the candidate though? | |||
Um, better question: do you look through the candidates list to see if it's there? Or just use it? | |||
If the former then the pointer'd not match | 23:20 | ||
If the latter, I've no guesses | |||
MasterDuke | just use it if not NULL | ||
in the MVM_SPESH_CHECK_PRESELECTION block i look through the list to find its index and compare it to the result of MVM_spesh_arg_guard_run | 23:21 | ||
gist.github.com/MasterDuke17/c0b51...6cf00808dc is the current diff on top of github.com/MoarVM/MoarVM/pull/1426 | 23:22 | ||
hm, don't think my logic is correct in frame.c | 23:28 | ||
yeah, i was never using the passed in spesh_cand | 23:33 | ||
now immediate segv | |||
but it was removing candidates before, so that part is working | 23:34 | ||
jnthn | Ah! Only with JIT, or always? | ||
MasterDuke | jit disabled | ||
yeah, this spesh_cand looks off | 23:38 | ||
$1 = {common = {header = {sc_forward_u = {forwarder = 0xffffffff00000003, sc = {sc_idx = 3, idx = 4294967295}, st = 0xffffffff00000003}, owner = 1, flags1 = 1 '\001', flags2 = 2 '\002', size = 24}, st = 0x5555555939f8}, body = { | |||
Ā Ā Ā cs = 0xffffffff00000003, type_tuple = 0x18020100000001, discarded = 192 '\300', bytecode_size = 21845, bytecode = 0xffffffff00000003 <error: Cannot access memory at address 0xffffffff00000003>, handlers = 0x18020100000001, | |||
Ā Ā Ā spesh_slots = 0x555555593b88, num_spesh_slots = 3, num_deopts = 4294967295, deopts = 0x18020100000001, deopt_count = 1431911504, deopt_named_used_bit_field = 18446744069414584323, deopt_pea = {materialize_info = 0x18020100000001, | |||
Ā Ā Ā Ā Ā materialize_info_num = 93824992492824, materialize_info_alloc = 18446744069414584324, deopt_point = 0x18020100000001, deopt_point_num = 93824992493024, deopt_point_alloc = 18446744069414584327}, num_inlines = 1, | |||
Ā Ā Ā inlines = 0x555555593ea8, local_types = 0xffffffff00000007, lexical_types = 0x18020100000001, num_locals = 16240, num_lexicals = 21849, work_size = 21845, env_size = 7, num_handlers = 4294967295, jitcode = 0x18020100000001, | |||
Ā Ā Ā deopt_usage_info = 0x555555594038}} | |||
MVM_frame_invoke was called from the interpreter's sp_fastinvoke_o | 23:40 | ||
jnthn | Uff, yeah. Do you MVMROOT the spesh_cand in the case there's a heap allocated frame? | 23:42 | |
MasterDuke | gist.github.com/MasterDuke17/c0b51...h-L97-L103 ? | 23:43 | |
jnthn | The interp.c changes look legit at least | 23:44 | |
ah, yes | |||
MasterDuke | hm, but i need to add spesh_cand to every MVMROOT above there, don't i? | 23:45 | |
jnthn | Yeah, but also I'm tired so maybe not reading this correctly but I'm not sure where you end up using the provided spesh_cand? | 23:46 | |
MasterDuke | gist.github.com/MasterDuke17/c0b51...patch-L112 | 23:47 | |
jnthn | Yeah, but that's conditional on: | 23:48 | |
if (chosen_spesh_cand_index >= 0 && chosen_spesh_cand_index < (MVMint32)spesh->body.num_spesh_candidates && cands_and_arg_guards) { | |||
I don't think that comes out true if we are given a spesh_cand already? | |||
Or rather, it only happens if MVM_SPESH_CHECK_PRESELECTION is turned on, I think | 23:49 | ||
MasterDuke | i just changed that to `if (spesh_cand || (chosen_spesh_cand_index >= 0 && chosen_spesh_cand_index < (MVMint32)spesh->body.num_spesh_candidates && cands_and_arg_guards)) {` | ||
`if (!spesh_cand) spesh_cand = cands_and_arg_guards->spesh_candidates[chosen_spesh_cand_index];` | |||
jnthn | aha, ok | 23:50 | |
There's probably a neater factoring but I think that's correct at least | |||
MasterDuke | huh, adding it to the other MVMROOTs in MVM_frame_invoke didn't change anything | ||
yeah, that will be cleaned up | 23:51 | ||
jnthn | Spesh slot marking shouldn't be the issue | ||
MasterDuke | btw, gist updated with what i'm currently running | 23:52 | |
jnthn | And the setup of it looks right in optimize.c | ||
Does it ever succeed with a passed-in spesh_cand? | 23:57 | ||
Are we looking at something occasionally wrong with it, or always wrong with it? | |||
MasterDuke | i think always wrong, this segv is very early | 23:59 |