github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
01:00 dogbert17 left 01:27 lucasb left 04:40 statisfiable6 left, sourceable6 left, linkable6 left, evalable6 left, bisectable6 left, tellable6 left, unicodable6 left, quotable6 left, squashable6 left, committable6 left, notable6 left, releasable6 left, shareable6 left, benchable6 left, nativecallable6 left, greppable6 left, bloatable6 left, coverable6 left, reportable6 left, nativecallable6 joined 04:41 quotable6 joined, squashable6 joined, benchable6 joined, linkable6 joined, shareable6 joined, evalable6 joined, notable6 joined 04:42 statisfiable6 joined, bloatable6 joined, sourceable6 joined, committable6 joined, reportable6 joined, greppable6 joined, releasable6 joined 04:43 bisectable6 joined, unicodable6 joined, coverable6 joined, tellable6 joined 04:52 discoD left 06:02 farcas1982regreg joined 06:09 AlexDaniel left 06:22 MasterDuke joined
nwc10 good *, #moarvm 06:48
07:31 farcas1982regreg left 07:37 patrickb joined 07:52 zakharyas joined
nine dogbert17: I think my heap profiler segfault is related to yours. It's also about a gen2 object that got at least part of it overwritten 08:17
tellable6 nine, I'll pass your message to dogbert17
nine It tries to process a string from a comp unit's string heap. The string itself is already freed from gen2, but the comp unit itself isn't 08:41
The string gets freed _after_ getting added to the comp unit's strings list, both of which seems to happen within the same GC run. 08:56
nwc10 nine: so you're making progress on debugging your crazy problem? 08:57
MasterDuke huh, the branchlut2 code at github.com/miloyip/itoa-benchmark is actually a little slower than the naive version. wonder if that's just one my architecture...
when i use it in our MVM_coerce_i_s that is 08:58
nine nwc10: indeed. It's starting to make sense. The heap profiler creates a snapshot as part of gc_finish. It gets the file name for a static frame from a bytecode annotation. That file name obviously lives in the comp unit's string heap and gets added to the comp unit's list of deserialized strings. 09:04
Those strings get allocated in gen2 by default, because comp units usually are long lived and so are their strings (and maybe other reasons).
But, we're currently in gc_finish. And only after the heap snapshot gets taken do we free the unmarked objects. This includes our newly born string, as it wasn't alive yet in the marking phase 09:05
MasterDuke heh. just saved 100m instructions off my benchmark, but still 100m more than the naive version 09:06
09:10 sena_kun joined 09:13 Altai-man_ joined 09:16 sena_kun left
MasterDuke am i imagining things or is 64 way bigger than needed here github.com/MoarVM/MoarVM/blob/mast...rce.c#L204 ? the worker code uses 20 github.com/MoarVM/MoarVM/blob/mast...rce.c#L176 09:22
m: say (2**63 - 1).chars 09:27
camelia 19
Geth_ MoarVM: 1aa555fe1d | (Stefan Seifert)++ | src/profiler/heapsnapshot.c
Fix segfault caused by freed comp unit strings when profiling

The heap profiler creates a snapshot as part of gc_finish. It gets the file name for a static frame from a bytecode annotation. That file name obviously lives in the comp unit's string heap and gets added to the comp unit's list of deserialized strings. Those strings get allocated in gen2 by default, because comp units usually are long lived and so are their strings (and maybe other ... (5 more lines)		 09:57
nine dogbert17: please give your profiler issue another try with github.com/MoarVM/MoarVM/commit/1aa555fe1d 10:01
tellable6 nine, I'll pass your message to dogbert17
10:09 zakharyas left, zakharyas1 joined
lizmat nine++ 10:14
10:24 dogbert17 joined
dogbert17 nine: ok, let me know if there's anything I can do to help 10:28
tellable6 2020-04-17T08:17:27Z #moarvm <nine> dogbert17: I think my heap profiler segfault is related to yours. It's also about a gen2 object that got at least part of it overwritten
2020-04-17T10:01:13Z #moarvm <nine> dogbert17: please give your profiler issue another try with github.com/MoarVM/MoarVM/commit/1aa555fe1d
10:38 MasterDuke left
dogbert17 starts testing the fix from nine++ 10:39
nine: perhaps there are more bugs (or I messed up): gist.github.com/dogbert17/e08f50c7...9d547e0199 10:43
nine Well there could be similar bugs 10:46
10:47 AlexDaniel joined, MasterDuke joined 10:48 AlexDaniel left, AlexDaniel joined 10:56 patrickb left
timotimo nine: nice find! 10:57
nine dogbert17: can you apply this patch and run it with a break point on the fprintf? gist.github.com/niner/176100dec8a7...0c4ca0b88a 10:59
That should tell us whether it's a similar bug or something different
11:15 sena_kun joined 11:17 Altai-man_ left 11:19 camelia left 11:26 camelia joined
dogbert17 nine: will do 11:35
dogbert17 makes some coffee 11:36
Geth_ MoarVM: ca6534d202 | (Stefan Seifert)++ | 2 files
Fix deadlock when trying to report an unsupported NativeCall return type
nine Mmmmmh...coffee
nwc10 was thinking about coffee too. 11:40
lizmat would love to have a cuppa, but alas can't cope with the cafeine anymore :-( 11:41
MasterDuke is just finishing the after lunch beer. will probably need an after beer coffee soon
lizmat resorts to a muckefuk 11:42
nine lizmat: there's pretty decent decaf nowadays
dogbert17 nine: am I supposed to apply your patch with 'git apply <diff-file>' ? 11:43
nine or patch -p1
jnthn Think I already had enough cups of coffee today...
nine I switched to coffee in the morning and black tea in the afternoon 11:44
Geth_ MoarVM: MasterDuke17++ created pull request #1272:
Use smaller buffer for native int to str coercion		 11:45
dogbert17 git apply fails with 'fatal: corrupt patch at line 66' with is after the last line in your gist
s/with/which/ 11:46
jnthn nine: Normally chamomile (is that how you spell it in English) or mint tea for me, but yeah, that's about what I do.
OK, I have the rest of the (working) day for MoarVM/Raku stuff :)
lizmat whee! 11:47
nine dogbert17: try adding a line with just a space at the end of the diff file. May have gotten lost in copy&pasting
dogbert17 nine: thx
nwc10 jnthn: is the beer fridge ready to celebrate success? 11:48
nine If everything else fails you can just do those changes manually.
MasterDuke jnthn, nine, et al.: while everyone is here, any thoughts on github.com/Raku/nqp/pull/612 ? is the change desired? and if so, how's the actual PR? 11:51
jnthn nwc10: Beer fridge is at home and I am not. :) But yes, there's beer there. :)
Even has a Tsarina Esra and a Yeti to choose from... 11:52
MasterDuke: Hm, that has append semantics on an existing buffer? 11:54
MasterDuke the hobgoblin imperial ruby ale i just had was pretty good. but i'll admit i was surprised at how minimal the selection of good craft beers is here in england (at least in the supermarkets i've been to). i've been told there's a good beer store nearby, but i only found out about it right before the lockdown...
jnthn: yeah, it requires github.com/MoarVM/MoarVM/pull/1267 . but that part could easily be removed if undesired by itself 11:55
jnthn The craft beer store here is doing delivery during lockdown, so I put in an order.
dogbert17 nine: for some odd reason it doesn't seem to hit the fprintf line you added?
jnthn MasterDuke: Yes, the MoarVM bit is what I was looking at
I guess it's quite useful
nine dogbert17: that would indicate, that it's a different type of bug than the one I fixed 11:56
dogbert17 takes a short break in order to watch the daily human malware press conference 12:00
Geth_ MoarVM: 6546027a1c | (Jonathan Worthington)++ | src/core/exceptions.c
Improve handling of errors in native callbacks

There's native code on the callstack, and we can't expect to end up in a consistent state if we just try and unwind over it via `longjmp`- even were that safely possible for the VM itself, which I don't know it is. Thus, termination is appropriate. However, the way we did that so far was a panic about stack unwinding, which provides no useful information ... (5 more lines)		 12:37
MoarVM: 05c290522d | (Daniel Green)++ | src/strings/ops.c
Allow nqp::encode() to take a preallocated buffer

Previously it would throw. Now if you give it a preallocated buffer it will just append to the end of it. This could be useful when you want to code a bunch of stuff into one big buffer since you won't have to create a bunch of temporary buffers and splice them all together (e.g., when creating the string heap in NQP).		 12:52
MoarVM: 0ce4cbe94f | (Daniel Green)++ | lib/MAST/Nodes.nqp
Simplify write_s since encode can append to a buf
MoarVM: ab3f19251c | (Jonathan Worthington)++ (committed using GitHub Web editor) | 2 files
Merge pull request #1267 from MasterDuke17/let_encode_take_a_preallocated_buffer

Let encode take a preallocated buffer
lizmat hmmm... all of this tastes like in need of a bump ? 12:54
jnthn Well, if we want to merge the NQP part of MasterDuke++'s work then we need to bump NQP to the latest MoarVM first. 12:55
Just looking at that now
MasterDuke: Travis seems unhappy with the PR 12:56
Is that just the missing MoarVM change?
Re-running them having merged the MoarVM git. 12:57
*bit
MasterDuke yeah, "encode requires an empty array"
dogbert17 nine: this looks a bit suspicious github.com/MoarVM/MoarVM/blob/mast...ots.c#L218 13:00
there are plenty of tests above that worklist is not null which seems to imply that it CAN be null 13:01
heh, worklist is actually set to NULL by the calling code 13:08
like so: github.com/MoarVM/MoarVM/blob/mast...hot.c#L681 13:11
13:14 Altai-man_ joined
jnthn MasterDuke: Yeah, it passes on MoarVM master now, so we should just do a bump in NQP and then can merge it' 13:15
13:17 sena_kun left
nine dogbert17: yeah that looks like it could be it 13:19
dogbert17 nine: is the proper fix to add a test around line 218 or is something else supposed to be done if worklist is NULL 13:22
nine dogbert17: adding that test should be fine. There doesn't seem to be a describe function we could call instead 13:23
lizmat lost what should be merged / bumped in what order 13:24
nine Though this probably means that we're missing information about those spesh plugin guards in the heap profile
dogbert17 will that cause problems? 13:27
well, the SEGV seems to be gone 13:36
nine I don't think so. At worst it will just miss some memory usage or shortest paths. Much less of an issue than segfaults... 13:37
I'd say commit that change, take your winnings and proceed with what you actually wanted to use the profiler for :)
Btw. I can only recommend reviewing open PRs to all developers regardless of current experience level. The reviews I've done today were both in areas where I had almost no prior exposure and now I have. And as a bonus I found an actual issue with one of them and maybe a way to improve the other one. 13:40
It can never hurt to take a look and if you're not a 100 % sure, you can still leave your feedback and leave the actual merging to more experienced or more daring developers :) 13:42
dogbert17 nine: will do :) 13:50
14:02 greppable6 left 14:33 greppable6 joined 15:15 sena_kun joined 15:16 Altai-man_ left
Geth_ MoarVM: dogbert17++ created pull request #1273:
Fix SEGV in MVM_spesh_plugin_guard_list_mark		 15:25
MoarVM: bceb3398a2 | (Daniel Green)++ | src/core/coerce.c
Use smaller buffer for native int to str coercion

There can only be up to 20 chars, and the actual worker functions only use 20 char arrays also.		 15:33
MoarVM: bb27aeb111 | (Daniel Green)++ | src/core/coerce.c
Don't null-terminate string when stringifying int

MoarVM doesn't use null-terminated strings, so this is unneeded.
MoarVM: 57592e616d | niner++ (committed using GitHub Web editor) | src/core/coerce.c
Merge pull request #1272 from MasterDuke17/use_smaller_buffer_for_native_int_to_str_coercion

Use smaller buffer for native int to str coercion
MasterDuke nine++ 15:58
so much merging going on. anybody have thoughts about github.com/MoarVM/MoarVM/pull/1270 ?
dogbert17 nine, I believe that I managed to change the incorrect sentence 16:09
nine dogbert17: now it's 3 commits?github.com/MoarVM/MoarVM/pull/1273/commits 16:10
dogbert17 uh oh, i did a git commit --amend. How do I fix this mess? 16:11
nine You can cherry-pick a258dca4e3997eed3c3fc6da7300d59b25465a63 into your master branch and submit a PR from that
dogbert17 a new PR then 16:12
Geth_ MoarVM: dogbert17++ created pull request #1274:
Fix SEGV in MVM_spesh_plugin_guard_list_mark		 16:18
MoarVM: dcbdb571a2 | (Jan-Olof Hendig)++ | src/gc/roots.c
Fix SEGV in MVM_spesh_plugin_guard_list_mark

MVM_gc_root_add_tc_roots_to_worklist contains several tests checking that the worklist parameter is not NULL. Unfortunately one call, to MVM_spesh_plugin_guard_list_mark, had slipped through the cracks. This could lead to a SEGV when running the heap profiler since it can call MVM_gc_root_add_tc_roots_to_worklist with a NULL worklist which in turn was used when calling MVM_spesh_plugin_guard_list_mark. Fixed by adding a check which ensures that worklist is not NULL before calling MVM_spesh_plugin_guard_list_mark.		 16:23
MoarVM: 60850e772d | niner++ (committed using GitHub Web editor) | src/gc/roots.c
Merge pull request #1274 from dogbert17/fix-heap-prof-segv

Fix SEGV in MVM_spesh_plugin_guard_list_mark
nine MasterDuke: I don't think you'll get more useful feedback on github.com/MoarVM/MoarVM/pull/1270 16:24
MasterDuke: let's just merge it and see how it fares.
timotimo yeah it's probably fine 16:52
hm. are sp_ ops really going in there?
i think the instrumentation happens before spesh runs? 16:53
jnthn Yes, I think so too 16:54
I don't think it's reinstrumetned after
I'm making quite a bit of progress on the dispatcher design, by the way.
Been thinking about it on and other for the last couple of weeks, but only today has it started to feel like it's falling into place. 16:55
MasterDuke timotimo: there were some pre-existing sp_* ops, so i added some more. are you saying they don't make sense? or something else? 16:59
timotimo i'd have to look into that more closely 17:00
until then it'd be barely negative
MasterDuke negative?
timotimo well, adding more cases to the switch that never get hit
is just maximum-*shrug* 17:01
MasterDuke ok. well, i'll just merge as is then and a subsequent cleanup can get them all
Geth_ MoarVM/master: 5 commits pushed by (Daniel Green)++, MasterDuke17++ 17:02
17:05 zakharyas1 left 17:07 zakharyas joined 17:14 Altai-man_ joined 17:17 sena_kun left 18:18 zakharyas left 19:15 sena_kun joined 19:17 Altai-man_ left 19:36 dogbert17 left 21:14 Altai-man_ joined 21:17 sena_kun left 23:15 sena_kun joined 23:17 Altai-man_ 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!