github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
nine The object gets moved by the GC as a spesh log entry's param.type. At that point MVMProfileCallNode referring to it as alloc->type already exists and seems to be part of tc->prof_data's call_tree. Still no idea why it didn't get marked as such at that point. 09:33
nine Actually there's a bit of a trap in rr: when you reverse-execute a program it will not clear memory past a malloc, i.e. data written to that memory block later on will already be there before the malloc. So my assessment that the MVMProfileCallNode already exists as is part of the call_tree may be wrong. 09:51
I haven't got definitive proof of that yet, as the node is several dozen layers deep in the call_tree and traversing that manually doesn't sound attractive. 09:52
Ah, there's rwatch for trapping that read. And...it doesn't fire. But with that I can probably bisect the predecessor chain to find out at which point it misses. 09:55
It's between ((MVMProfileCallNode *) 0x6471420)->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->num_succ 09:58
and ((MVMProfileCallNode *) 0x6471420)->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->num_succ
Err ((MVMProfileCallNode *) 0x6471420)->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->pred->num_succ rather
nine OMG it is so blaringly obvious! 10:19
jnthn I was about to ask if there's a cat on your keyboard... :) 10:20
nine How on earth can almost a century of combined programming experience miss this? 10:21
jnthn :)
jnthn looks forward to the commit
nine We reallocate the list to make more space: github.com/MoarVM/MoarVM/blob/mast...ment.c#L23
But we never add the node to the list in that case!
jnthn Oops 10:23
nine I immediately spotted the harmless off-by-one which is usually hard to find. I saw the unused code. But this simple beginner's mistake escaped me :)
Geth_ MoarVM: 81f9ccdfa5 | (Stefan Seifert)++ | src/profiler/instrument.c
Fix profiler's call graph node getting missed by the GC

When the NodeWorklist hit its allocated size, add_node would reallocate the list but then not actually add the node to the resized list. This made us miss this node during the GC's mark run and led to outdated object pointers in the allocation list.
... (5 more lines)
10:55
nine Is there a ticket for this? 10:58
MasterDuke nine++ 11:42
Guest23744 nine++, I believe that there are several issues relating to crashing/broken profiles 12:23
perhaps they have been fixed by your commit
for example M#1023 12:24
synopsebot M#1023 [open]: github.com/MoarVM/MoarVM/issues/1023 SEGV during a --profile
MasterDuke nine, timotimo: i can now complete a profile of the mqtt test file that's always failed before. however, when i open it in moarperf i see `Error: near line 11: out of memory` and the .sql file seems broken. it's 223mb, but only 19 lines 12:25
nine Guest23744: yes, looks likely 12:26
MasterDuke that's the one i'm talking about
and yeah, even trying to open the .sql in sqlite on the command line gave the out of memory error (and i have 32gb) 12:27
nine I also think there's an issue in the profiler in general and that disabling inlining helps sort of confirms this. Because why would a call_graph even be more than 256 levels deep in a one liner program? Yes it involves EVAL and the compiler really does create pretty large call stacks. But > 256 is somewhat excessive. 12:28
So I'm quite sure that there's still an inlining related callstack growth issue in the profiler. That would probably also explain the extreme memory usage in those examples and the huge profile sizes. 12:29
MasterDuke yeah, that test files runs much much faster when inlining is disabled (while profiling_ 12:31
Guest23744 M#1019 12:37
synopsebot M#1019 [open]: github.com/MoarVM/MoarVM/issues/1019 Code generates huge broken profile
Guest23744 R#3006 12:38
synopsebot R#3006 [open]: github.com/rakudo/rakudo/issues/3006 Crash while generating profiling data.
jnthn nine++ 12:39
Guest23744 jnthn: you were right, you suspected add_node from the beginning 12:41
and now we're officially out of fromspace errors :-) 12:42
nine known ones at least :) 12:43
MasterDuke the example in R #3006 now completes, but it does create a large profile even when disabling inlining 12:48
but maybe it's legitimately large
MasterDuke does 12:59
Data::Dump::Tree using threading? or Dom::Tiny or Terminal::ANSIColor? 13:00
because a profile of that example says there were 6 threads active at the end 13:01
and the top 5 functions by inclusive are all Thread and ThreadPoolScheduler stuff 13:02
timotimo nine: thanks for spotting that, that was good 14:52
i should have just used MVM_VECTOR from the start
nine Oh, true, that's a thing! 15:02
timotimo well, i'm glad that a more competent engineer (namely nine) is now having a look at the "profile becomes huge when inlining" issue
one thing i have identified is that sometimes when doing a throwpayload that "does" a goto, it can skip a prof_leave call 15:03
maybe prof_enter and prof_leave should just write a value to a register and when doing prof_leave see if it matches, and if not just leave more until it's correct 15:04
like the address of the profiler node
MasterDuke timotimo: this look good to you? github.com/MoarVM/MoarVM/pull/1185 16:06
timotimo huh, i could have sworn i've put that in in the past 16:08
but yeah, i think that looks good 16:09
MasterDuke cool
Geth_ MoarVM: 9f24debe21 | (Daniel Green)++ | src/profiler/instrument.c
Check if subtracting a successor's exclusive...

time would cause the total exclusive time to underflow. If so, just set it to 0 instead.
MoarVM: 44811cab69 | MasterDuke17++ (committed using GitHub Web editor) | src/profiler/instrument.c
Merge pull request #1185 from MasterDuke17/fix_underflowing_exclusive_times_in_profiles

Check if subtracting a successor's exclusive...
MasterDuke timotimo: btw, any idea why gist.github.com/nkh/f8d41e0748c325...0b0f24f142 (minus the LWP::Simple call) would have 6 threads active? 16:18
timotimo no. try the debug env var for the thread pool scheduler maybe? 16:23
MasterDuke ah right, i knew there was something that would give me more info
RAKUDO_SCHEDULER_DEBUG_STATUS? 16:26
Altai-man_ RAKUDO_SCHEDULER_DEBUG? 16:28
MasterDuke hm, neither RAKUDO_SCHEDULER_DEBUG nor RAKUDO_SCHEDULER_DEBUG_STATUS seem to be useful here
Altai-man_ ah, and STATUS is valid one too, then ignore me
timotimo the status one is extremely spammy 16:30
MasterDuke i think part of the problem is that the profiling itself is causing some of the symptoms? 16:36
but i thought there was some env var that would log when threads get started and such. but RAKUDO_SCHEDULER_DEBUG creates a log that just says it's creating an affinity worker thread and a general worker thread 16:38
timotimo profiling shouldn't cause user code threads to be spawned unless the threadpoolscheduler already does something 18:13
Kaiepi valgrind's finally usable on openbsd amd64!! 18:22
timotimo \o/ 18:23
now there's no longer any need for anybody to use linux :\ 18:24
Kaiepi can someone review and merge github.com/MoarVM/MoarVM/pull/1166 at some point this week? 18:33
nine Kaiepi: I could have a look at the code but I'm not sure about the architectural parts. Having low level operations in the VM try multiple addresses before reporting any error to me sounds too magical for that layer. 18:39
Btw. there seem to be leftover debug statements: github.com/MoarVM/MoarVM/pull/1166...a3dcf9R325 18:40
Kaiepi the problem is trying only the first record makes it so you can't connect to addresses you're able to in other languages if it's ipv6 and ipv6 is misconfigured on the machine used 18:41
that was removed in one of the later commits i think
yeah 18:42
there aren't any printfs left over from while i was writing it 18:43
nine Oh, I clearly see the need for trying multiple addresses. I just wonder if that shouldn't be implemented in a higher layer, e.g. rakudo's IO::Socket::INET::!initialize 18:44
Personally I very much prefer reviews where the commits are cleaned up already, i.e. the commits should not reflect your development history with all the detours but should tell the story of how to get from the previous state of the repository to the new state in small, distinct and self-contained steps. 18:46
Kaiepi hm, like giving struct addrinfo a repr and letting rakudo handle them instead?
MasterDuke timotimo: even if i set RAKUDO_MAX_THREADS=1, the profile still says 6 threads active
Kaiepi yeah this was originally meant to be just the first 3 but i got kinda sidetracked 18:47
nine yes, that sounds like a way to do it
That way the user could easily override the default by subclassing.
timotimo could set a breakpoint on the "start" feature
nine I love the Linux kernel developer's mantra here: the kernel should provide mechanism, not policy. To try those addresses in the order they are returned is a policy. That's subject to change. That we get access to all the addresses is a mechanism that can be used to implement any policy. 18:48
MasterDuke timotimo: ah, good idea 18:49
nine Kaiepi: I now realized that I could have raised these concerns any time in the past few weeks but always thought that I'd have to spend considerable time to dig into the networking stuff first :/ Sorry for the delay 18:50
Kaiepi is that something that could be dealt with after i complete the grant? this wasn't originally part of it since i thought just making the socket op use families/socket types/protocols alone would fix what this addresses at the time of writing
nine I'd guess yes. I can also just turn this around: if you assure me that this policy thing can be raised to upper layers in a backwards compatible way after a merge, I'm fine with the PR :) 18:52
This way you won't have to do it now, just see a clear way how it can be done.
Kaiepi i can make a problem solving issue for it so i don't forget
timotimo MasterDuke: can you see any user code being executed on the other threads? 18:54
MasterDuke mostly no
timotimo you can also search the call graph for "start"
nine timotimo: what's parent_id in the calls table exactly?
timotimo that could give you the place where something's done
nine: points to another node in the calls table
Kaiepi one's probably needed anyway since if we're going to be making ops for dns-related stuff there are other ops that could be added as well
timotimo except i guess for the thread's root node
MasterDuke timotimo: can i send you a profile? 18:55
timotimo sure
MasterDuke it's 15mb compressed, how do you want it? 18:56
timotimo sharedrop.io? 18:57
www.sharedrop.io/rooms/802ab925-d8...68f7adedff
aha 19:00
Thread list <unit-outer> <unit> ddt get_dump get_dump_lines <anon> render_root reset QX shell spawn-internal start 19:01
MasterDuke ah 19:02
`my $width= %+((qx[stty size] || '0 80') ~~ /\d+ \s+ (\d+)/)[0] ;` 19:06
any better way to do that? 19:07
timotimo all ways we have to call out to programs will launch threads because they are all internally implemented with react blocks 19:08
(unless you use nqp ops) 19:09
MasterDuke wow. i hardcoded the output of `stty size` in my terminal in instead of the qx call. didn't really make anything faster. but `GC runs have taken 32.88% of total run time` 19:17
timotimo: i just got a big red frowny face 19:21
with lots of text in a hover-over that starts with`TypeError: routine is undefined` 19:22
dogbert17 is everyone debugging the profiler tonight? 19:42
timotimo: are you around? 19:47
have an interesting gist, profiling related, for you or anyone else who might be interested: gist.github.com/dogbert17/d6291bfb...61f56ba704 19:48
MasterDuke wow, lots going on there 19:49
dogbert17 yeah, something suspicious going on there 19:50
I am running with the very latest MoarVM as well, i.e. 44811cab69 19:52
there problem remains even if spesh is disabled 19:56
MasterDuke any different output if you build moarvm with --valgrind? 19:57
dogbert17 let me try
MasterDuke i can't repro btw 20:00
dogbert17 could be because I'm running with an 8k nursery and MVM_GC_DEBUG=1
nine timotimo: if you need to reliably detect when a frame was exited again, isn't that what MVMFrameExtra's special_return and special_unwind are for? 20:26
timotimo using special_return is maybe a bit expensive, and i'm not sure if special_return prevents inlining, it might want to 22:08
jnthn I think it's more like "nothing that can have special return can be inlined" 22:19
But that's no good for profiling, since "how much is inlined" is one of the most interesting things to find out when profiling :) 22:20
timotimo ayup 22:23