github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
00:01 vrurg joined 00:06 vrurg left 00:28 sena_kun left 00:32 vrurg joined 01:22 frost-lab joined 04:25 squashable6 left 04:28 squashable6 joined 05:07 squashable6 left 05:08 squashable6 joined 05:46 MasterDuke left
nwc10 good *, #moarvm 06:37
07:04 frost-lab left 07:18 xiaomiao left 07:20 xiaomiao joined 07:22 nine left 07:24 frost-lab joined 07:29 nine joined 07:35 frost-lab48 joined 07:38 sena_kun joined, frost-lab left 08:19 domidumont joined 09:15 zakharyas joined 09:20 nebuchadnezzar left 09:21 Altai-man_ joined, sena_kun left 09:27 nebuchadnezzar joined
nine MasterDuke: there are indeed other reasons for getting a NULL out of decont. E.g. getobjsc can return NULL instead of VMNull if an object doesn't have an SC 09:34
tellable6 nine, I'll pass your message to MasterDuke
jnthn That should really be fixed. 09:42
tellable6 2021-03-23T18:49:57Z #raku-dev <patrickb> jnthn: May I poke you to have a quick look at github.com/Raku/raku.org/pull/160 ?
jnthn (To return VMNull)
nine Have done so locally already. 09:43
Also rather more interesting cases, like Perl6::Actions::maybe_add_inlining_info using nqp::hllbool, but that's NQP code and NQP doesn't have hll bools, so nqp::hllbool returns NULL: github.com/rakudo/rakudo/blob/mast....nqp#L4364 09:44
I'll have it throw an exception in that case 09:47
As VMNull won't be a good boolification of a true value :)
jnthn :) 09:48
nine I just wonder. With `return 0 unless nqp::isnull($phasers) || !nqp::hllbool($phasers);` since nqp::hllbool would always return NULL there, doesn't this mean that we actually bailed out only if $phasers was specifically VMNull? 10:06
Argh...negations are hard. I meant we kept running only if $phasers was VMNull, ignoring other false values. 10:07
nwc10 nine: you did breakfast? and coffee? 10:08
10:08 lizmat left
jnthn nine: I think nqp::isnull may well check for C nulls too 10:09
nine After several minutes of thinking through that expression, I'm fair and square with Perl Best Practices that one should avoid `unless` or at least restrict it to a single, simple boolean. But...in this case the short circuiting property of || actually mattered.
jnthn Although I wish it didn't have to 10:10
nine jnthn: it does, since it just wraps MVM_is_null which is return !check || check == tc->instance->VMNull;
nwc10: oh breakfast? I knew I forgot about something :D 10:15
10:22 lizmat joined
nine Apparently there's only 1 spec test failure remaining. But that one's not due to a NULL or anything being wrong with my trivial 4 line fix. 11:19
11:20 frost-lab48 left
nine It's because test 157 in t/spec/S32-exceptions/misc.rakudo.moar:473 suddenly finds the fail sub itself in a Failure's backtrace. The fun part is: this test also fails on master when run with MVM_SPESH_DISABLE=1 11:20
Looks like the first test that actually relies on spesh
And not just on spesh but on the spesh bug that I fixed :) 11:21
nwc10 hangon. there might be two
that was one.
IIRC t/spec/S02-types/isDEPRECATED.rakudo.moar does too, but I can't remmeber why offhand 11:22
actually, IIRC t/spec/S02-types/isDEPRECATED.rakudo.moar was more that it doesn't pass with spesh set to NODELAY
nine The message for the commit that introduced the test ("[coverage] Cover Backtrace.flat") points at the test not actually be written as specification but as enshrinement of current implementation behaviour 11:23
nwc10 that other one it will pass under MVM_SPESH_DISABLE
IIRC that's what I concluded from that commit message
and I couldn't see what it was really trying to test
hence how to write a better test
or even if it was something that should have been tested
nine I really don't see a reason to omit sub fail in that backtrace. It's not marked hidden-from-backtrace and anyway, the test tests Backtrace::list which is precicely the way to get a full, unfiltered list of frames. 11:28
The irony is that the test is marked TODO on JVM which according to ^^^ actually behaves entirely correctly
11:32 frost-lab joined
dogbert12 clickbaits github.com/MoarVM/MoarVM/pull/1436 11:33
nine I merged 1436 11:36
dogbert12 nine++ 11:37
Only two fails when running a stresstest, t/spec/S17-procasync/kill.t and t/spec/S06-signature/introspection.t. 11:56
nine and for kill.t the fix is known 11:57
dogbert12 very cool
nwc10 nine: yes, you agree with my assessment - the JVM is correct
dogbert12 perhaps the other test failed because I didn't have the latest rakudo changes 11:58
11:58 zakharyas left
lizmat FWIW, I've just bumped MoarVM and NQP 12:01
dogbert12 lizmat++
14:00 Kaiepi left 14:01 Kaeipi joined, Kaeipi left 14:02 Kaeipi joined 14:11 zakharyas joined
nine I have to admit: Raku is really damn cool: is any($bt.list).code.name, 'foo', '.list contains correct items'; 14:13
nwc10: the remaining difference in backtrace frame count with differen MVM_SPESH_* options is due to inlining. MVM_exception_backtrace needs to use the frame walker to construct correct frame lists in the face of inlining 14:22
That fail() test is also a bit of a can of worms. We've already established that the test is incorrect as it only expects 4 frames which only happens due to the spesh bug. 14:33
On JVM and with spesh disabled it will get 6 frames instead. <unit>, the first anonymous block, the second anonymous block, sub foo, sub fail and... what's the sixth and why would there be one more? Turns out, the inner most is Failure.new which I claim is a bit unexpected. 14:34
But why is that there? Turns out it's only been there since 2019 and came with this commit: github.com/rakudo/rakudo/commit/96...11ee58c65a 14:36
The commit reduced the number of frames we skip when creating the backtrace from 5 to just 3.
Incidentally the tests were TODOed a mere 2 months after that change to the implementation. So before the change, they appear to have passed. 14:41
Btw. the test's description has always been wrong (tests for 4 elements but message is "we correctly have 2 elements in .list") since the test was introduced in 2016 except...for a very brief period in 2018 when it the test was changed to > 3 which and reverted less than an hour later. 14:48
nwc10 I vageuly remember figuring *some* of this stuff out. But, I had no idea what the right answer should have been, or even the right questions to ask someone else
and there was too much $other going on, so it fell on the floor 14:49
nine My conclusion is, that the test is actually very much correct. Expecting 4 frames does make sense: <unit>, block, block, sub foo. I could argue both for and against a frame for fail() itself, but since we've excluded it historically, I'd err on that side. The change from backtrace offset 5 to 3 needs reverting and finally, MoarVM needs to produce correct backtraces. 14:51
My spesh fix helps and finally bringing the frame walker to MVM_exception_backtrace will do the rest. 14:52
15:01 domidumont left 15:02 codesect` joined 15:04 codesections left 16:07 Kaeipi left, Kaeipi joined 16:09 Kaeipi left, Kaeipi joined 16:23 zakharyas left 16:42 Merfont joined 16:45 codesect` left 16:46 Kaeipi left
nine The yak goes even deeper than expected on this one. Turns out even the frame walker gets it wrong. Specifically when the JIT is enabled, it misses an inlined frame. 16:58
Oh boy... 17:08
I think, it works like this: we're in Backtrace.new which looks like this: try X::AdHoc.new(:payload("Died")).throw; nqp::create(self)!SET-SELF( nqp::backtrace(nqp::getattr(nqp::decont($!),Exception,'$!ex')), 1) 17:11
I.e. we create an exception in a try thunk and then generate the backtrace for this exception.
So we're currently in that nqp::backtrace which calls MVM_exception_backtrace which I've just converted to using the frame walker. 17:12
The frame walker uses MVM_jit_code_get_current_position which if the given frame is tc->cur_frame returns tc->jit_return_address. That's the current position of currently executing JIT code.
Now with inlining active, that try thunk surely gets inlined into Backtrace.new. This means that the nqp::backtrace op is in the same frame as the exception was thrown in, i.e. tc->cur_frame. 17:13
So the frame walker gets the point of the nqp::backtrace as the current position of JIT compiled code we're in. And at this position there is no inlined frame.
17:25 Merfont left, Merfont joined 17:28 Kaeipi joined, Merfont left 17:40 zakharyas joined 17:57 frost-lab left 18:50 Kaeipi left, Kaeipi joined 19:13 vrurg left 19:16 vrurg joined 19:21 zakharyas left 19:38 Kaeipi left, Kaiepi joined
nine The surprisingly simple solution: remove the JIT implementation of nqp::backtrace 19:46
TEST_JOBS=100 make stresstest -> All tests successful. 19:49
nwc10 OK, the implication of *having* a JIT implementation of backtrace was to make generating backtraces faster? 19:50
and I then have to ask "how often do people need backtraces in performance critical paths?"
nine Well nqp::backtrace is a very heavy weight op anyway and the JIT in this case can only remove the comparatively tiny OP dispatch overhead. 19:52
vrurg nwc10: sometimes it's the best source of information about the calling context. And it may theoretically be a performance-critical case.
nwc10 the combination of thoese two responses makes me think "heavy weight op", "best source of information about the calling context" - for code wanting that, how much of what the op produces is thrown away? ie - being able to implement it as more than one op woudl give spesh more to learn 19:54
and I'm not say "please someone do this"
vrurg But as I remember a discussion once held about it, the biggest problem with nqp::backtrace is that it is using an exception. And this is where it becomes most expensive.
nwc10++ 19:56
Perhaps something like nqp::frame(int $level) would be a good answer to many questions. 19:57
lizmat nwc10 nine nqp::backtrace is invoked for *every* Failure, afaik 20:02
nine I know 20:03
nwc10 oh.
nine It's what I'm workig on
nwc10 I don't seem to be adding value here.
lizmat nwc10: au contraire!
I too think there should be an easier, less heavy way for getting backtrace info 20:04
nine nwc10: I'd say you're right on target. We do need a faster way and decomposition may be helpful 20:05
nwc10 I continue to be impressed by how "Slower" things that can be picked apart by spesh can end up faster 20:06
nine I have an alternative that is removing that inlinable thunk from the equation: nqp::backtrace(nqp::handle(X::AdHoc.new(:payload("Died")).throw,'CATCH',nqp::exception)), 20:14
It's about as fragile as the other option. 20:15
21:10 Kaiepi left 21:11 Altai-man_ left, Kaeipi joined 21:26 MasterDuke joined 21:42 zakharyas joined
MasterDuke nine++ you're on a roll. i'd also done some exploration into t/spec/S32-exceptions/misc.rakudo, but didn't get as far as you did 21:44
tellable6 2021-03-24T09:34:43Z #moarvm <nine> MasterDuke: there are indeed other reasons for getting a NULL out of decont. E.g. getobjsc can return NULL instead of VMNull if an object doesn't have an SC
21:44 zakharyas left 21:48 Kaeipi left 21:50 Kaiepi joined 22:33 dogbert17 joined, dogbert12 left 23:02 dogbert11 joined 23:06 dogbert17 left

Powered by Cro and the Raku® Programming Language.

Please report any issues / comments / feature requests as an issue on App::Raku::Log.

Thank you!