MasterDuke | it looks like 3rdparty/libtommath is just added to the moar repo, instead of pointing to a specific commit at github.com/libtom/libtommath. it also hasn't been updated to the 1.0.0 release | 00:20 | |
timotimo | i think we also have at least one patch that upstream doesn't have | 00:21 | |
MasterDuke | ah, i see 5 commits to that directory after the import | 00:25 | |
timotimo: any idea why this code would use a ton of ram? `my ($x,$y,$i) = (0,1,1); while ($i < 200000) {$y += $x ; $x = $y - $x ; $i += 1 } ; say $y;` | 00:57 | ||
it does create a string that's 41799 chars long, but just doing `my $y = "y" x 41799` is much smaller | 00:58 | ||
it does create a cstring, then MVM_string_ascii_decode()s it to an MVMString, and then boxes is to a Str. but /usr/bin/time is showing 1129028maxresident, compared to 67732maxresident | 01:01 | ||
01:39
Ven joined
|
|||
timotimo | i'll compile a vmhealth-capable moar and check it out | 01:43 | |
but it'm in need of some sleeps | |||
okay, so | 01:49 | ||
it doesn't do a single gen2 collection | |||
it thinks at the end that gc_bytes_promoted_since_full is 6389445 | |||
it does allocate 753 pages in the 4th slot of the gen2 | 01:50 | ||
oh, ok, if i reduce the limit by a whole lot, that stays the same | 01:51 | ||
MasterDuke | oh, so maybe if some other work is done also that triggers a gen2 collection the end number would go down? | 01:52 | |
timotimo | maybe | ||
it's not impossible that we don't count big ints into the total byte amount very well? | |||
though actually those big ints should all die quickly | 01:53 | ||
at first i thought we were outputting the result of the while loop | |||
so i thought we had a big array of Int objects that could be the culprit | |||
MasterDuke | what would trigger a gen2 collection? | 01:55 | |
timotimo | when the "bytes since last full" have exceeded a certain threshold | 01:56 | |
MasterDuke | i just tried heaptrack, it says mp_grow ( github.com/MoarVM/MoarVM/blob/mast...grow.c#L19 ) allocated 1Gb | 01:59 | |
timotimo | well, the unmanaged_size function just returns the bigint object's "alloc" entry | ||
perhaps we have to multiply that by some factor, not sure how exactly mpint works for that field | 02:00 | ||
MasterDuke | oh, it actually allocated 3.6Gb total (if i'm reading this correctly), that 1Gb was just a peak | 02:01 | |
timotimo | in short, all i can say is *shrug* until i look much further into this | ||
right, "max"residentk | 02:02 | ||
would probably be interesting when exactly it reaches the peak | |||
MasterDuke | thanks for the help, was checking that rt.perl.org/Ticket/Display.html?id=126450 is still valid | 02:04 | |
timotimo | it allocated 1gb total, or is that the biggest allocation it has tried? | 02:07 | |
mp_grow i mean | 02:08 | ||
MasterDuke | 3.6G total allocated, 1G peak | ||
timotimo | does that mean "memory allocated here has peaked, in total, at 1 gigabyte"? or "the call to this that allocated the most so far allocated 1gb"? | 02:09 | |
MasterDuke | don't know | ||
i.imgur.com/EuGds7O.png | 02:10 | ||
i ran with --full-cleanup | 02:11 | ||
timotimo | OK | ||
so how much memory does it take to store that result value once? | |||
m: say "{138847 / 8} bytes" | 02:12 | ||
camelia | 17355.875 bytes | ||
timotimo | so a single instance of these numbers would cost us ~17kb | ||
794396maxresident without the "say" at the very end | 02:14 | ||
MasterDuke | i profiled just creating the number that was printed directly and then say()ing it again. 1.5G total | 02:15 | |
according to heaptrack | |||
02:16
Ven joined
|
|||
timotimo | how did you create that? | 02:16 | |
just put it into the source? | |||
MasterDuke | the command line. my terminal complained though | 02:17 | |
timotimo | interesting. the number of "bytes promoted since last full" hits a constant very early on | ||
what do the colors mean in the graph btw? | |||
MasterDuke | 66748maxresident according to /usr/bin/time | ||
timotimo | i bet the "sizes" tab is interesting, too | ||
MasterDuke | different functions | 02:18 | |
the orange is mp_grow. the next largest is mp_init_size | |||
i.imgur.com/jF8hZ7K.png | 02:19 | ||
timotimo | a bit strange that it would skip buckets like that | ||
though they are basically logarithmic | |||
okay, i'm getting too tired to continue this | 02:20 | ||
but it seems like we're doing rather well compared to when this took 6 gigs | |||
on the other hand it can't have been tuning the threshold for doing a gen2 collection | |||
because it never even does a gen2 collection and it comes out ahead | |||
did we do something wrong in the code that makes it blow through the nursery faster? | 02:21 | ||
MasterDuke | well, i've been testing with 200000, the ticket was for 1000000 | ||
timotimo | oh | ||
MasterDuke | i tried 1000000 and the OOM killer struck | 02:22 | |
timotimo | it just reached 6 gigs resident memory for me | ||
i do have 2 gigs of swap, though | 02:23 | ||
(zram) | |||
MasterDuke | i have 8gb, but no swap | ||
timotimo | i have 16 gigs main ram | ||
yeah, it peaks at 6228292maxresident | |||
i suppose the problem is this: | 02:24 | ||
we never collect before the nursery is full | |||
i don't know the exact number but you can fit really many big ints into a nursery | 02:25 | ||
this code does hardly anything but build lots and lots of big ints | |||
1885160maxresident | 02:27 | ||
^- this is what happens when i run 1_000_000 but do a nqp::force_gc every 10_000 | |||
MasterDuke | oh interesting | ||
timotimo | 983480maxresident force_gc every 5_000 | 02:28 | |
MasterDuke | yeah, heaptrack looks very different with force_gc every 5_000 | 02:29 | |
timotimo | 261824maxresident force_gc every 1_000 | ||
02:31
Ven joined
|
|||
timotimo | 111332maxresident - every 500 | 02:32 | |
running every 50 now | 02:33 | ||
83828maxresident - every 50, but also threw out the vmhealth printing code | |||
maybe moar needs to gain another pressure value that gets upped when really really big objects like strings, arrays, bigints, get allocated | 02:34 | ||
so we do minor collections more often, too | |||
anyway, i think my hypothesis has gotten a lot of proof, and i'm willing to say that's what is the cause | |||
i need sleep :| | |||
have a good one! | |||
MasterDuke | thanks, and likewise | 02:35 | |
.tell samcv fyi, heaptrack says MVM_unicode_name_to_property_code is leaking with the code i was testing with: `my ($x,$y,$i) = (0,1,1); while ($i < 200000) {$y += $x ; $x = $y - $x ; $i += 1 } ; say $y;` | 02:37 | ||
yoleaux2 | MasterDuke: I'll pass your message to samcv. | ||
samcv | thx for the heads up MasterDuke | ||
yoleaux2 | 02:37Z <MasterDuke> samcv: fyi, heaptrack says MVM_unicode_name_to_property_code is leaking with the code i was testing with: `my ($x,$y,$i) = (0,1,1); while ($i < 200000) {$y += $x ; $x = $y - $x ; $i += 1 } ; say $y;` | ||
samcv | working on something atm but will look at it later today | 02:38 | |
MasterDuke | np | ||
02:48
ilbot3 joined
|
|||
samcv | MasterDuke, also interesting it's leaking with that code? | 03:43 | |
you aren't checking the properties ricght? | |||
i guess it might still happen | 03:44 | ||
MasterDuke | right, wasn't doing anything unicode-y | ||
i also tried reverting to before your most recent commit, but it was still the same | |||
samcv | my last commit has nothing to do with that one | 05:34 | |
06:18
geekosaur joined
06:20
geekosaur joined
06:22
geekosaur joined
07:02
zakharyas joined
|
|||
timotimo | it has always been leaking forever and ever | 07:52 | |
we just don't have refcounting or something for the unicode db | 07:53 | ||
so even full_cleanup doesn't clean it up | |||
or am i confusing this with something else ... | |||
08:19
dalek joined
08:20
synopsebot6 joined
08:28
synopsebot6 joined
08:34
domidumont joined
08:40
domidumont joined
09:23
zakharyas joined
09:44
FROGGS joined
10:12
Ven joined
11:15
Ven joined
|
|||
MasterDuke | timotimo, samcv: i added a print statement in MVM_unicode_name_to_property_code and i see it called 6 times if i run the code above, alternately looking up 'No' and 'Nl' | 11:57 | |
ah, golfed it to`my ($, $) = 1`; for every element in the lhs list, there's a lookup of 'No' and 'Nl' | 12:00 | ||
timotimo: about the memory usage of the code we were playing with, should i create a MoarVM issue? or is the RT sufficient | 12:03 | ||
IOninja | What happens when a function is only in the header file? For example, I'm trying to find what MVM_file_isreadable does, but `grep -R 'MVM_file_isreadable' .` gives me just src/io/fileops.h where it's declared and src/core/interp.c where it's used. How to get to code that gets executed when it's used? | 12:25 | |
MasterDuke | maybe run in the debugger with a breakpoint on that function name? | 12:27 | |
IOninja | no idea how to do that :} | 12:28 | |
MasterDuke | i think just `break MVM_file_isreadable` in gdb. i'll give it a try, but i'm no debugger expert | 12:29 | |
src/io/fileops.c:321 | 12:30 | ||
IOninja | oh a macro | 12:31 | |
MasterDuke++ thanks | |||
MasterDuke | looks like it calls `file_info` | ||
np | 12:32 | ||
13:27
gk_1wm_su joined
13:40
Ven joined
13:52
ZofBot_ joined
13:53
ZofBot joined
14:06
Ven joined
14:36
Util joined
17:40
dogbert17 joined
|
|||
dogbert17 | m: react { Supply.interval(1).tap: -> { .say } } # RT 127098 | 17:42 | |
camelia | Unhandled exception in code scheduled on thread 4 | ||
dogbert17 | my attempts to find useful info wrt the 'decoderstream' bug have failed spectacularly, amusing myself browsing old RT's instead | 17:44 | |
jnthn | Useless use of react... The exception is because of the zero-arg closure | 17:58 | |
So the code is wrong | |||
But it's LTA we aren't saying that the exception was | |||
Oh, but | |||
m: Supply.interval(1).tap: -> { .say }; sleep 1 | 17:59 | ||
camelia | Unhandled exception in code scheduled on thread 4 Too many positionals passed; expected 0 arguments but got 1 in block at <tmp> line 1 |
||
jnthn | We *do* | ||
But the main thread exits before we have chance to print the backtrace :) | |||
So this code (a) wrongly used react by doing tap not whenever inside of it, (b) wrongly used tap too | 18:00 | ||
I can't think of much practical we can actually do here | 18:02 | ||
Short of trying to build some kind of "don't let the main thread exit if another is reporting an exception" | 18:03 | ||
MasterDuke | jnthn: different topic, but re bigints and nurseries and allocations, i assume you don't mean check whether every MVM_(m|re|c)alloc is above a certain size, and if so adjust the nursery? | 18:04 | |
jnthn | No, that's be very costly :) | 18:09 | |
And won't actually help | |||
Because iirc libtommath won't call MVM_malloc anyway | 18:10 | ||
MasterDuke | ha, i was pretty sure that wasn't what you meant | ||
jnthn | I think when we produce a new bigint (that's actually big), before we hand it back to the user, we take the number of digits and use that to influence the nursery size | 18:11 | |
MasterDuke | no, looks like it calls a lot of realloc in mp_grow | ||
dogbert17 | jnthn: thx for the explanation, I guess the that's the same problem which crops up when using Proc::Async to run a non existing command ... | 18:12 | |
jnthn | So, in src/math/bigintops.c | ||
dogbert17: I thought that was a panic? | |||
Which is rather different | |||
And if you're running a process with proc::async you should be awaiting it somewhere | |||
dogbert17 | jnthn: I was thinking about stuff like this, gist.github.com/dogbert17/8b285d91...7ebe30452f | 18:14 | |
one might think that the gisted code should print out some kind of error message but it does not | 18:16 | ||
dogbert17 but perhaps that is another problem entirely | |||
jnthn | That looks unreleated, yeah | ||
MasterDuke | jnthn: "we take the number of digits and use that to influence the nursery size", that's what we do now, or what we should do? | ||
dogbert17 | should it print some kind of error msg, as it is now nothing is pronted out | 18:17 | |
s/pronted/printed/ | |||
jnthn | MasterDuke: That's what I'm suggesting we (carefully) do | ||
dogbert17: Yes, the await should throw | 18:18 | ||
Due to a broken Promise | |||
Note that on Windows it behaves differently badly | |||
dogbert17 is on a Linux VM | |||
jnthn | Yeah, I can replicate what you see on Linux too | 18:19 | |
dogbert17 | if run through gdb it breaks with a SIGPIPE | ||
jnthn | OK, jalfrezi is ready; bbl :) | ||
dogbert17 | Indian food again :) | ||
jnthn: I updated the gist above with gdb output, there's indeed a broken Promise in there | 18:23 | ||
jnthn | Ohh...so we're missing the signal? Odd | 18:58 | |
Ah, right...seems it needs doing explicitly | 19:00 | ||
timotimo | we had pancakes <3 | 19:06 | |
dogbert17 | jalfrezi sounds like the winner there :) | ||
jnthn, is it a difficult fix? | 19:07 | ||
jnthn | In theory,no | ||
dogbert17 | is it a MoarVM problem or a rakudo one? | 19:08 | |
jnthn | We could in theory fix it in Rakudo, but in practice it's likely better done in Moar | 19:09 | |
dogbert17 | I believe I have seen several RT's describing this or some very similar problem | ||
jnthn | In theory just doing `signal(SIGPIPE, SIG_IGN);` or so at MoarVM startup would do it | 19:10 | |
dogbert17 | the word theory crops up again :) | 19:11 | |
jnthn | Well | ||
I'm pretty confident that fixes it for us on, say, Linux, OSX, etc. | 19:12 | ||
I'm not sure it'll work on Windows | |||
dogbert17 | so the handling on Win is totally different | 19:13 | |
should I report it as a MoarVM issue, referencing relevant RT's if I can find them? | 19:17 | ||
jnthn | Sure | 19:18 | |
I think we can add the thing I suggested in a #ifdef check to make sure we're not on Win32 | 19:19 | ||
dogbert17 | cool | ||
RT #125651 seem to be one of them | 19:20 | ||
synopsebot6 | Link: rt.perl.org/rt3//Public/Bug/Displa...?id=125651 | ||
jnthn | Also there is a MoarVM issue already filed for what happens on Windows | 19:22 | |
dogbert17 | and this perhaps RT #128526 | 19:26 | |
synopsebot6 | Link: rt.perl.org/rt3//Public/Bug/Displa...?id=128526 | ||
19:36
Ven joined
21:50
lizmat joined
22:19
Ven joined
22:33
geekosaur joined
22:39
Ven joined
|
|||
MasterDuke | timotimo: i think github.com/MoarVM/MoarVM/commit/152337ab0c broke some tests. some things are now isbig_I that weren't before | 22:56 | |
t/spec/S05-transliteration/trans.t, t/spec/S32-list/combinations.t, t/spec/integration/advent2009-day08.t | 22:57 | ||
with a couple dying here: github.com/rakudo/rakudo/blob/nom/...Int.pm#L65 | |||
timotimo | um. wat. | 23:01 | |
isbig_I is not for that purpose | 23:02 | ||
at least i don't think so | |||
the commit that added it didn't say anything interesting :\ | 23:03 | ||
MasterDuke | i think the combinations.t failure was essentially combinations(3.5, 2) should equal combinations(3, 2) | 23:05 | |
but instead i got: First parameter out of range. Is: 3, should be in -Inf^..2147483647 | |||
that was here: github.com/rakudo/rakudo/blob/nom/...or.pm#L491 | 23:07 | ||
timotimo | *sigh*, feel free to revert my revert, then. | 23:11 | |
MasterDuke | your moar commit? i don't have a commit bit. or you mean make a PR to revert it? | 23:13 | |
timotimo | i'll revert it | 23:31 | |
Geth | MoarVM: 5dd8f04b1a | (Timo Paulssen)++ | src/math/bigintops.c Revert "throw out flawed parts of bigint_is_big" This reverts commit 152337ab0c17e242ff69b245c3cb7b5954fe42d0. Turns out some tests rely on this. I don't think nqp::isbig_I should be used in lieu of checking bounds, but what do I know. |
23:32 | |
MasterDuke | should those rakudo bits really be changed to not use isbig_I? | 23:42 | |
timotimo | jnthn will probably know better than me | 23:43 | |
Geth | MoarVM: MasterDuke17++ created pull request #546: Check result of repeat/concat fit in an MVMString |
23:56 |