MasterDuke | timotimo++ the heaptrack graphs after your rand_I commit look much nicer (for `perl6 -e 'srand(1); say [+] ^1000 .roll(1_000_000)'`) | 01:27 | |
timotimo | thought so. screenshots, though? :D | 01:31 | |
MasterDuke | before: i.imgur.com/NzSzHUW.png after: i.imgur.com/TTqz45D.png | 01:50 | |
most of the graphs look pretty similar. steady growth before, plateauing at a much smaller value after (or still steady growth, but with a much smaller max) | |||
01:53
ilbot3 joined
02:11
bisectable6 joined,
quotable6 joined,
unicodable6 joined,
bloatable6 joined,
committable6 joined,
coverable6 joined,
evalable6 joined,
greppable6 joined,
benchable6 joined,
statisfiable6 joined
02:27
lizmat joined
|
|||
timotimo | holy wow the time difference is amazing | 02:34 | |
jesus christ, the difference of max is what 100x? | 02:35 | ||
it's allocations in total, though, so parts of that get freed again of course | |||
08:21
pharv joined
09:04
edehont joined
|
|||
nwc10 | timotimo: I configured MoarVM with `Configure.pl --ubsan --asan ...` so I assume it's more that I didn't turn *on* the leak checker, instead of "I turned it off". | 09:28 | |
but yes, I think that that means that there is no leak checker, so no excitement generated even if stuff leaks | |||
10:19
colomon joined
10:34
colomon joined
|
|||
MasterDuke | timotimo: the op name for coverage options: controlcoverage, coveragecontrol, control_coverage, coverage_control? or do you have any other name suggestions? | 11:39 | |
11:40
dogbert2 joined
|
|||
jnthn | MasterDuke: If it's user-facing, we don't put _ in op names except for type suffixes at the end | 11:46 | |
timotimo | i'd like coveragecontrol | 11:47 | |
jnthn | We also have prefixes like sp_ for non-public ops used for the VMs internal purposes | ||
timotimo | sp for spesh | ||
MasterDuke | it's user facing, so coveragecontrol it is | 11:48 | |
nwc10 | lunch& | 11:51 | |
12:11
robertle joined
13:16
nwc10 joined
14:22
AlexDaniel joined
|
|||
nine | I'm writing a realloc helper that initializes the new memory with 0 (could call it recalloc) for fixing serialization reading uninitialized memory. Should I put it alongside MVM_realloc as MVM_recalloc? | 15:16 | |
timotimo | yeah why not | 15:22 | |
Geth | MoarVM: MasterDuke17++ created pull request #626: Add nqp::coveragecontrol op |
15:26 | |
MasterDuke | timotimo: how's ^^^ look? | 15:27 | |
timotimo | huh | 15:29 | |
explain the implementation in interp.c to me? | |||
oh | |||
of course, it shall only work if the env var is set | |||
MasterDuke | yeah | 15:30 | |
timotimo | the first argument to coveragecontrol should be r instead of w | ||
i thought we'd have a separate value for the env var for "no deduplication of log lines", or is there no use for the thing "in the middle"? | 15:31 | ||
MasterDuke | updated to r | 15:32 | |
timotimo | better | ||
(rather than bettew) | |||
MasterDuke | "mawwiage is what bwings us hewe today" | 15:33 | |
timotimo: wasn't sure about the de-dup thing | |||
and the various use cases | 15:34 | ||
timotimo | i think it's a valid use case to not want dedup | ||
MasterDuke | have to not de-dup when using coveragecontrol() of course | ||
timotimo | i mean, you can subtract startup if you don't dedup, but since we also call coveragecontrol after startup you can just use that, too | 15:36 | |
but maybe you want to see how many times individual parts are run | |||
MasterDuke | could add an option to coveragecontrol just to turn off de-dup | ||
timotimo | aye | ||
MasterDuke | has anyone done a coverity scan recently? might be a good idea after all the recent big changes | 15:51 | |
timotimo | oh, i haven't thought of that in a long time | 15:52 | |
Geth | MoarVM: aa740f6329 | (Stefan Seifert)++ | src/6model/serialization.c Ensure that serialized padding bytes are 0 Helps creating reproducible build of precompiled Perl 6 modules |
16:00 | |
MoarVM: 0304b316dd | (Stefan Seifert)++ | src/6model/serialization.c Avoid uninitialized bytes in default tables These tables are all cretaed with reasonable default sizes but it's quite possible that they are not fully utilized in the end. So we need to initialize them with an equally reasonable default value. Helps creating reproducible build of precompiled Perl 6 modules. |
|||
MoarVM: 4ce573d9c5 | (Stefan Seifert)++ | 2 files Avoid uninitialized bytes at the end of serialized tables |
|||
16:14
geekosaur joined
|
|||
MasterDuke | timotimo: do you think it ever makes sense to turn off de-duping in the middle of a script/program? or should it be an all-or-nothing option controlled by the env variable? | 16:15 | |
timotimo | i imagined one-or-nothing | ||
16:18
zakharyas joined
|
|||
MasterDuke | something like MVM_COVERAGE_CONTROL=2 means don't de-dup, but don't wait for an nqp::coveragecontrol(1) to start logging? | 16:19 | |
and MVM_COVERAGE_CONTROL=1 means don't de-dup, but *do* wait for an nqp::coveragecontrol(1) to start logging? | |||
16:20
travis-ci joined
|
|||
travis-ci | MoarVM build failed. Stefan Seifert 'Avoid uninitialized bytes at the end of serialized tables' | 16:20 | |
travis-ci.org/MoarVM/MoarVM/builds/261335193 github.com/MoarVM/MoarVM/compare/6...e573d9c568 | |||
16:20
travis-ci left
|
|||
MasterDuke | t/hll/06-sprintf.t (Wstat: 0 Tests: 286 Failed: 2) Failed tests: 285-286 Parse errors: Bad plan. You planned 284 tests but ran 286. | 16:23 | |
16:28
http_GK1wmSU joined
|
|||
MasterDuke | nine: ^^^ | 16:29 | |
nine | MasterDuke: ah, I can reproduce it locally. I should really adopt the habit of running nqp's tests, too | 16:31 | |
16:31
http_GK1wmSU left
|
|||
nine | But I somehow doubt that the failure is due to my commits: Parse errors: Bad plan. You planned 284 tests but ran 286. | 16:32 | |
Indeed, reverting nqp commit 980a07ee96216a43958d54878e187a7b0abd8cf1 fixes the test | 16:34 | ||
Pushed a fix for the test plan. It was really just 2 new tests. | 16:36 | ||
MasterDuke | nine++ | 16:42 | |
16:54
Ven joined
17:07
pharv joined
17:11
benchable6 joined,
committable6 joined,
statisfiable6 joined
|
|||
Geth | MoarVM: ff571d17fd | (Stefan Seifert)++ | src/core/alloc.h Fix "error C2036: 'void *' : unknown size" Thanks to ugexe++ for reporting |
18:36 | |
MasterDuke | there seems to be some memory leak/growth after 2017.07 | 18:40 | |
2017.07: i.imgur.com/nx2EGjw.png HEAD: i.imgur.com/8USB7cx.png | 18:41 | ||
18:58
travis-ci joined
|
|||
travis-ci | MoarVM build passed. Stefan Seifert 'Fix "error C2036: 'void *' : unknown size" | 18:58 | |
travis-ci.org/MoarVM/MoarVM/builds/261366040 github.com/MoarVM/MoarVM/compare/4...571d17fd17 | |||
18:58
travis-ci left
|
|||
jnthn | MasterDuke: Is that really what the data is saying? | 18:59 | |
The line it's highlighting is: | 19:01 | ||
void *tospace = MVM_calloc(1, MVM_NU | |||
u | |||
void *tospace = MVM_calloc(1, MVM_NURSERY_SIZE); | |||
ilmari | nine: -Werror=pointer-arith would have caught that | 19:02 | |
jnthn | The reason we allocate a lot more there now is because instead of memsetting a ~4MB region, we just free it and calloc a new one | ||
Because measurements showed that to be cheaper than the memset | |||
callgrind thought a vast amount cheaper; actual time measurements showed rather less | 19:03 | ||
But yes, this will mean that our "churn" in terms of allocation calls will be a lot higher | |||
MasterDuke | hm, makes sense | ||
19:04
Ven joined
|
|||
jnthn | Given that the memory in question is fromspace that will be cache cold by then, though, then I don't think there's much chance that calloc will do a slower job than we can with memset | 19:04 | |
MasterDuke | AlexDaniel thinks the whateverables are slower and leakier than they were recently | 19:05 | |
jnthn | Yeah | ||
I think the top change it points to isn't the one we're after, alas | |||
What code is that btw? | 19:06 | ||
MasterDuke | k, any ideas where to look? | ||
jnthn | And why on earth is it decoding a million ASCII strings | ||
MasterDuke | that i heaptracked? | ||
jnthn | Yeah | ||
A million allocations in ascii decode must mean a million strings decoded | |||
Which is, uh...possible curious | 19:07 | ||
MasterDuke | `perl6 -I Text-Diff-Sift4/lib/ -e 'use Text::Diff::Sift4; my $d; for ^1000000 { $d = sift4(~$_, "10") }; say $d'` | ||
jnthn | *but curious | ||
Hm, is it calling .decodee somewhere in there? :) | |||
MasterDuke | nope | ||
jnthn | Oh, it's doing a million iterations | 19:08 | |
MasterDuke | github.com/MasterDuke17/Text-Diff-.../Sift4.pm6 | ||
jnthn | So it's only doing it once I guess | ||
such, I don't see what in there would be doing it either | 19:09 | ||
Oh | 19:10 | ||
~$_ | |||
It'll be that :) | |||
Turning a number into an MVMString | |||
OK, mystery solved :) | |||
Does this code leak btw? | |||
Or just show more churn? | |||
MasterDuke | well, /usr/bin/time for 1000 iterations shows 82264maxresident)k, 1000000 iterations shows 95516maxresident)k | 19:12 | |
jnthn | That could just be the GC finding its natural overhead and/or spesh logs/stats/optimized code using a bit of space | 19:13 | |
MasterDuke | yeah, 10m iterations is 95704maxresident)k | ||
jnthn | Yeah | ||
walk; bbiab | 19:14 | ||
Geth | MoarVM: ilmari++ created pull request #627: Add -Werror=pointer-arith for gcc |
19:16 | |
MasterDuke | timotimo: any thoughts about my last MVM_COVERAGE_CONTROL=* comments? | 19:40 | |
timotimo | ah yes | 19:41 | |
that's how i imagined it | |||
MasterDuke | cool, i'll see about adding that to the PR | 19:42 | |
timotimo | great! | ||
thanks a lot | |||
MasterDuke | heh, likewise, you did the hard part of actually implementing line coverage | 19:43 | |
timotimo | you mean "the easy part", right? :) | 19:44 | |
MasterDuke | right, bet it was a breeze | 19:45 | |
i.imgur.com/7DOfHQt.png | 19:47 | ||
re memory stuff, does ^^^ look more interesting | 19:48 | ||
it's from running this gist.github.com/MasterDuke17/685b6...09c6a777a4 | 19:50 | ||
/usr/bin/time for 10 iterations shows 1773636maxresident)k, 20 iterations shows 2635308maxresident)k | 19:52 | ||
github.com/MoarVM/MoarVM/blob/mast...ops.c#L509 is what heaptrack says is the biggest allocator (after the calloc in gc we talked about earlier) | 19:55 | ||
timotimo | i wonder what the suggested size would turn out to be? | ||
is it perhaps wildly overestimated? | 19:56 | ||
MasterDuke | it's always 65536 | 20:00 | |
AlexDaniel | MasterDuke: so Sift4 by itself does not leak? | 20:08 | |
MasterDuke | doesn't seem like it | 20:09 | |
AlexDaniel | uhhhhh | 20:10 | |
then it has to be something here github.com/perl6/whateverable/blob...#L205-L231 | 20:11 | ||
timotimo | MasterDuke: could you check what buf->len and nread are in async_read when it repr_push_o the newly created buffer object into "arr"? | ||
that way we could figure out if we keep much-too-big buffers around instead of maybe reallocing them to shrink 'em | |||
AlexDaniel | it leaks tens of megabytes per one execution of get-similar, so the leak should be associated with the data | 20:12 | |
MasterDuke | line 531? | ||
Geth | MoarVM: ece9788ee0 | (Dagfinn Ilmari MannsƄker)++ | build/setup.pm Add -Werror=pointer-arith for gcc This disables the GNU extension that allows arithmetic on pointers to void and functions, which standard C (and MSVC) don't allow. Also remove redundant -Wdeclaration-after-statement: -Werror=foo implies -Wfoo. |
||
timotimo | around there, yeah | 20:13 | |
MasterDuke | buf->len is always 65536, nread ranges from 41 to 11767 | 20:14 | |
timotimo | okay, if we don't shrink them at all we'll be leaving a lot of memory on the table | 20:15 | |
however | |||
we're likely to just immediately decode the buffers, i believe | |||
so they should actually be thrown away quickly | |||
AlexDaniel | ok I can reproduce it | 20:16 | |
gist.github.com/AlexDaniel/5763d0b...893b7e2a85 | 20:17 | ||
if my math is correct, it's about 128 MB per call | 20:19 | ||
unfortunately I have to go nowā¦ | |||
timotimo | thanks for the effort | 20:20 | |
AlexDaniel | well, I'll get back to it soon | ||
get-similar only needs get-output, which is essentially just ārunā | |||
so this looks golfable | 20:21 | ||
anyway, cya | |||
timotimo | MasterDuke: you think it'd be a good idea to realloc these bufs to a number closer to the number of bytes read? do these buffers stick around? | 20:22 | |
MasterDuke | no idea | 20:25 | |
i think this is the first time i've looked at procops.c | 20:27 | ||
20:28
Ven joined
|
|||
MasterDuke | could try and see what happens. where should they be realloced? | 20:28 | |
timotimo | just before they get pushed there | 20:30 | |
same location i pointed at before | 20:31 | ||
20:35
dogbert11 joined
|
|||
MasterDuke | the buffer is created via MVM_repr_alloc_init? | 20:38 | |
`res_buf->body.ssize = buf->len;` should be `res_buf->body.ssize = nread;` instead? | 20:39 | ||
jnthn | No, because otherwise the heap profiler can't account for it properly | 20:44 | |
ssize is meant the be the allocated amount, which is what buf->len is | |||
elems is the amount actually in use | |||
libuv seems at least with sockets to always tell us a quite big amount even if it in reality only puts some hundreds of bytes in there, though. There's quite possibly a good reason for that. | 20:45 | ||
MasterDuke | is there something wrong in the perl6 code we're running? | 20:46 | |
which is essentially this: gist.github.com/MasterDuke17/685b6...09c6a777a4 | 20:47 | ||
jnthn | Not so far as I can see. | 20:48 | |
Huge leak though o.O | |||
I wonder if it's related to the one I've noticed with an async socket server in a react block | |||
Planning to investigate that one some more next week | 20:49 | ||
No particular idea what is going on | |||
MasterDuke | sounds similar | ||
20:51
colomon joined
|
|||
MasterDuke | any pointers where to look? | 20:56 | |
21:00
colomon joined
|
|||
Geth | MoarVM: dogbert17++ created pull request #628: Remove unused vars |
21:07 | |
21:07
channing1 joined
|
|||
dogbert11 | MasterDuke: tried your gist, although I ran fewer iterations. Get the impression that it leaks ~10k per iteration | 21:37 | |
that leak seems to involve newly added functionality | 21:39 | ||
MasterDuke | huh, pretty sure i'm seeing more than that. wonder if it's the 32vs64 bit? | ||
dogbert11 | could be, and perhaps the fact that the jit doesn't work in 32 bit might have something to do with it | 21:40 | |
AlexDaniel | so does it leak or not? :) | ||
ah, I see | 21:41 | ||
ok | |||
MasterDuke++ | |||
would be nice to have a ticket with this | 21:42 | ||
as far as I know, this is a regression | |||
at least, I'm 90% sure that it was working OK before the most recent changes | 21:43 | ||
MasterDuke | AlexDaniel: do you see any difference if you disable spesh or jit? | 21:45 | |
AlexDaniel | I see no difference | ||
anyway, later :) | 21:46 | ||
dogbert11 | MasterDuke: your gist contains this line which is commented out, # flat(@options, @tags, @commits).min: { sift4($_, $tag-or-hash, 5, 8) } | 21:49 | |
MasterDuke | yeah, it's adapted from whateverable code | 21:50 | |
dogbert11 | and you get a big leak without it? | ||
MasterDuke | yeah, that's commented out in the "real" code also | 21:51 | |
dogbert11 | valgrind doesn't find any big leaks but using /usr/bin/time I see that maxresidentk gets higher the more iterations I run | 21:52 | |
it's around 0.5gig after 20 iterations | 21:53 | ||
almost as if the GC isn't doing what one might expect, odd | 21:56 | ||
timotimo | you can ask the telemetry log how often gc runs if you're worried it doesn't do enough | 21:58 | |
MasterDuke | i updated my gist with the output of running for 5 iterations with valgrind --leak-check=full | 21:59 | |
big jumps in amount lost here: gist.github.com/MasterDuke17/685b6...g-L95-L174 | 22:01 | ||
dogbert11 | yeah, 66k definitely lost | ||
most of what is 'new' comes from spesh stats/plan related stuff. The unicode thing have been there for ages :) | 22:03 | ||
MasterDuke | valgrind output looks the same even with an nqp::force_gc() in the loop | 22:04 | |
dogbert11 | would it be possible for the amount of memory used by spesh to run amok or are there some kind of limit in place? | 22:06 | |
jnthn | If MVM_SPESH_DISABLE=1 then it never allocates a log buffer to write into, so the stats stuff never gets data... | 22:19 | |
So if it leaks with MVM_SPESH_DISABLE=1 then it rules out quite a bit. | |||
MasterDuke | the numbers are a little smaller, but still regularly increasing with MVM_SPESH_DISABLE=1 | 22:21 | |
jnthn | Yeah, then it's something not spesh, almost certainly | ||
There have been GC changes but... | |||
Hard to imagine them introducing leaks :S | 22:22 | ||
MasterDuke | dinner & | ||
dogbert11 | the amount of memory used is quite high though, this is from a 10 iteration run: | ||
==20843== in use at exit: 138,096,415 bytes in 437,172 blocks | |||
==20843== total heap usage: 1,566,554 allocs, 1,129,382 frees, 4,591,697,481 bytes allocated | 22:23 | ||
definitely lost: 64,928 bytes in 3,183 blocks | |||
22:24
d33pb00k-GK1wmSU joined,
d33pb00k-GK1wmSU left
|
|||
dogbert11 | doing the ten iterations with MVM_SPESH_DISABLE=1 leaks 'definitely lost: 1,256 bytes in 29 blocks' | 22:36 | |
half of that amount comes from MVM_decoder_configure (Decoder.c:131) | 22:37 | ||
dogbert11 has problem seeing the problem :) | 22:41 | ||
timotimo | got an idea how often that gcs in total? | 22:42 | |
might be able to get some insight from a heap snapshot | |||
MasterDuke | i get `Command terminated by signal 11` if i add --profile=heap | 22:43 | |
timotimo: how do you turn on telemeh? | |||
35 'gc finished' in the telemetry log for a 5 iteration run | 22:46 | ||
timotimo | that's not terribly much | 22:52 | |
MasterDuke | timotimo: PR updated | 23:37 | |
timotimo | looks good i think! | 23:44 | |
MasterDuke | cool | 23:45 | |
timotimo | anything good to find in the output? | 23:46 | |
also, i wonder when we should change the output format to be separated by something easy-to-find | 23:47 | ||
like nul bytes | |||
MasterDuke | heh, i had some actual usecase, but now have spent so long messing with getting it working i don't remember what it was | 23:50 | |
timotimo | we can figure out what the hot spots are in a program, and probably also an exact trace through the program | 23:51 | |
files are gonna be huge, though | |||
MasterDuke | yep and yep | 23:52 | |
will add a column to coverable6's output for hit count | 23:54 | ||
timotimo | mhm | 23:55 | |
got an idea what the performance impact is? | |||
MasterDuke | somewhat minimal, but i think the only time i measured it wasn't a very long or cpu intensive program anyway | 23:57 | |
timotimo | i have a lock there, right? that'd serialize multiple threads? | 23:58 | |
MasterDuke | there where? | 23:59 | |
timotimo | i don't seem to, no |