Welcome to the main channel on the development of MoarVM, a virtual machine for NQP and Rakudo (moarvm.org). This channel is being logged for historical purposes.
Set by lizmat on 24 May 2021.
MasterDuke [Coke]: what are the modules you use in your script that fails with mimalloc? 14:33
[Coke] some combination of File::Directory::Tree, File::Find, ile::Temp, HTTP::UserAgent, JSON::Tiny, Text::CSV 14:48
*File::Temp
MasterDuke may have found something in nativecall-related code. or might not be thinking straight, not sure how i could have missed it before 14:56
[Coke] Happy to test something if you throw it in a branch 14:59
pretty sure HTTP::UserAgent has some nativecall in there for SSL
MasterDuke arg, now getting github errors when trying to push 15:04
well, here's the patch until github fixes itself pastebin.com/5pcuNjEe 15:08
timo do libraries use glibc malloc and try to free() something nativecall allocated for them with mimalloc-malloc? 15:48
MasterDuke yep 16:10
Geth MoarVM: MasterDuke17++ created pull request #1686:
Correctly allocate/free CStrs when using mimalloc
16:11
lizmat MasterDuke: should that be in the 2022.03 release ? 17:39
[Coke] I need to test it on my windows issue, but that's probably not a blocker if it looks correct otherwise. 17:42
nine Yes, we want that fix 17:43
MasterDuke: why make the fix conditional on MVM_USE_MIMALLOC? I'd say that using MVM_malloc/MVM_free has never been correct there and just worked by virtue of those being wrappers for malloc/free. 17:44
[Coke] MasterDuke: going through rebuild now, will report out in a bit. 18:58
[Coke] MasterDuke: it's gotten *much* further using mimalloc+your path 19:24
Patch
nice find!
Yup, we're good 19:26
Nicholas "good" - is this like my COVID test result - "NOT DETECTED" ? :-/ 19:29
(unrelated to COVID-or-not, still rather too tired/busy to review things) 19:30
[Coke] yes, I can't reproduce my very difficult to golf error with his patch applied. 19:44
(oof, it's still running, up to 840MB so far...) 19:48
so this seems slower. 19:51
Finally did end, but got up to 1G of memory - which to be fair I'd not been checking previously. 19:52
MasterDuke nine: just to minimize the changes compared to before mimalloc. if we want to remove the use of MVM_* in these sorts of cases i think it'd make sense to do that as one larger commit across the different nativecall reprs 20:03
[Coke]: i'm kind of surprised if you find using mimalloc slower. pretty much everything i've tested has been faster with it, especially so on windows 20:04
japhb Perhaps [Coke] meant "slower memory growth"? 20:06
[Coke] no, I meant the script seemed to be taking longer to run 20:30
MasterDuke that's unexpected, but i don't think my patch would cause any noticeable change. have you found using mimalloc generally slower? 20:32
but CI is green and it fixes your issue, so i'm going to merge in a couple minutes unless there are any objections 20:39
[Coke] MasterDuke: I haven't been *using* mimalloc because it was broken. 20:40
so I can't really answer that.
MasterDuke ah 20:41
[Coke] I run these scripts daily or so: If it's really slower, I'll try to do a comparison this weekend when I can set it to run and not worry about it.
lizmat MasterDuke: when merged, will you bump also ? 20:52
MasterDuke sure 20:53
Geth MoarVM: 764fff9b86 | (Daniel Green)++ | src/6model/reprs/CStr.c
Correctly allocate/free CStrs when using mimalloc
21:05
MoarVM: c935e681b8 | MasterDuke17++ (committed using GitHub Web editor) | src/6model/reprs/CStr.c
Merge pull request #1686 from MasterDuke17/more_nativecall_mimalloc_fixes