timotimo | i was sidetracked by people around me | 00:08 | |
samcv: the problem is that we're joining a whole bunch of things and we have to check every seam where we join stuff | 00:09 | ||
i have probably hallucinated the result where i had a tenth of what i thought i'd have | 00:11 | ||
okay, i probably have to assume my problem is that i recurse instead of doing my stuff flat? | 00:14 | ||
one of the strings has 7201 pieces | |||
so it could very well be that at its peak it's 7k calls down | 00:15 | ||
in its heap | |||
okay, cool, now i'm "down" to 4 gigs | 00:17 | ||
MasterDuke | look at those gains! | 00:18 | |
timotimo | dem gains, brah | ||
parse-string is just not so clever in general | 00:19 | ||
00:20
Ven joined
|
|||
timotimo | it may want a partial rewrite | 00:20 | |
MasterDuke | do you know what "\n" or "\r\n" are as MVMGrapheme32's? | 00:21 | |
timotimo | yup | 00:22 | |
MVM_nfg_crlf_grapheme(tc) | |||
i made it work much faster, but now it b0rks multiple \ in a row | 00:23 | ||
no, something more basic was actually wrong | 00:28 | ||
grrr | |||
i think i'll drive home instead of continuing to bang my head against this | |||
no doubt this can be done much easier | |||
wat, i just made it pass?! | |||
haha. nope. | |||
MasterDuke | may have made it a bit faster and use less memory, nqp tested fine, seeing what a spectest looks like | 00:31 | |
in MVM_nfg_is_concat_stable, changed this `if (last_a < 0 || first_b < 0)` to this `if ((last_a != MVM_nfg_crlf_grapheme(tc) && last_a < 0) || (first_b != MVM_nfg_crlf_grapheme(tc) && first_b < 0))` | 00:33 | ||
spectest passed, hope something would have failed if my change wasn't correct | 00:34 | ||
timotimo | i expected something like that would be right | 00:35 | |
but it could very well be that our test coverage of this is ... meager | 00:36 | ||
MasterDuke | if i printed the .perl of the from-json result, would that be different if i'd messed something up? | 00:37 | |
timotimo | not necessarily | ||
it's probably only bad in cases we're not thinking of right now | 00:38 | ||
there's also the comment above that if that says that the rest of the code relies on the last_a and first_b being >= 0 | 00:39 | ||
MasterDuke | but it says that's an over-estimate, right? | ||
timotimo | yes | 00:40 | |
but maybe we have to put checks for < 0 into the other pieces of code | |||
bbiab | |||
MasterDuke | well, i'll PR it for people to think about | ||
fwiw, it was about .5s faster and .3g less ram | 00:41 | ||
Geth | MoarVM: MasterDuke17++ created pull request #556: Special case "\r\n" in MVM_nfg_is_concat_stable |
01:07 | |
timotimo | hm. that's not tremendously much | 01:21 | |
the memory usage is still pretty amazing | |||
in a bad way | 01:23 | ||
dat profile.sql ... | 01:30 | ||
01:34
MasterDukeMobile joined
|
|||
MasterDukeMobile | Yeah, the qt viewer choked when I created a .json profile | 01:35 | |
Too deeply nested or something like that | |||
timotimo | could be, yeah | ||
the plot sqlitebrowser shows for retained/promoted/cleared bytes is ... amazing | 01:36 | ||
cleared is extremely low all the time | |||
MasterDukeMobile | Hm, haven't tried a profile after my change | ||
timotimo | and the other two make a trumpet | ||
oh, your change. i should pull that in | |||
the BEGIN and COMMIT in the .sql file make sqlitebrowser quite confused | 01:38 | ||
it seems to want to put its own transaction around the code | |||
which then explodes because you can't have nested transactions in sqlite | 01:39 | ||
the other way to do it makes it fail to create it somehow and it just waits forever | |||
well, sqlite3 /tmp/profile.sqlite -init /tmp/profile.sql does the trick just fine | 01:41 | ||
MasterDukeMobile | Please feel free to edit that code (sql profile stuff) as you see fit | ||
timotimo | i was still hoping you could do that one thing for me :3 | 01:43 | |
anyway, the ping-ponging between promoted and retained is a clear indicator of having almost completely long-lived objects | |||
i feel like i did something today | 01:46 | ||
that's nice | |||
MasterDukeMobile | Yep | 01:49 | |
timotimo | i think it'd be really valuable if parse-string wouldn't recurse | 01:50 | |
MasterDukeMobile | Recursion can be so clean a solution and yet not so great for performance | 01:53 | |
timotimo | mhm | 01:54 | |
MasterDukeMobile | Btw, did you pull my change? If so, did it change the profile much? | ||
timotimo | really difficult to judge | 01:59 | |
were you able to spot if the difference in memory usage was mostly re_nfg? | |||
MasterDukeMobile | Not sure. But I think it dropped to approx 35% in the perf report | 02:00 | |
timotimo | it used to be almost 50%? | 02:01 | |
MasterDukeMobile | I think I compared at the same commit of Json::fast if that's what you mean | ||
Yep | |||
49 -> 46 with the first j::f fix, then to 35 with my change | 02:03 | ||
Something like that anyway | |||
MasterDuke | just looked at the previous heaptrack report. 890m allocated in re_nfg, 432m after my change | 02:33 | |
02:48
ilbot3 joined
07:24
domidumont joined
|
|||
samcv | yay | 07:26 | |
gonna pull MasterDuke | |||
merge i mean | |||
Geth | MoarVM: f273133979 | (Daniel Green)++ | src/strings/nfg.c Special case "\r\n" in MVM_nfg_is_concat_stable "\r\n" can never combine with anything else, so we can special-case it in the check whether the relevant graphemes are synthetic. |
07:27 | |
MoarVM: fe1dc84ac7 | (Samantha McVey)++ | src/strings/nfg.c Merge pull request #556 from MasterDuke17/special_case_crlf_in_MVM_nfg_is_concat_stable Special case "\r\n" in MVM_nfg_is_concat_stable |
|||
08:54
KDr2_c joined
09:06
domidumont joined
09:11
domidumont joined
09:28
domidumont joined
09:31
domidumont joined
09:35
agentzh joined
11:03
Ven joined
11:13
Ven_ joined
|
|||
timotimo | i wonder if we can only re_nfg in join when we're finished | 11:33 | |
instead of over and over and over | |||
lizmat | jnthn: re docs.google.com/spreadsheets/d/1kp...edit#gid=0 , why is .elems delegating to .cache, end end to .list ? I mean, isn't end just .elems - 1 ? | 11:35 | |
MasterDuke | timotimo: MVM_string_join does only do it to the final result `return MVM_nfg_is_concat_stable(tc, a, b) ? result : re_nfg(tc, result);` | 12:08 | |
timotimo | oh | 12:13 | |
strange | |||
why then would it take so long? :\ | 12:14 | ||
MasterDuke | timotimo, jnthn: slight change in topic, in native_ref_fetch (src/6model/containers.c:234) or native_ref_fetch_i, is there any way to tell if we're dealing with a signed vs unsigned native? it switches on a MVMNativeRefREPRData->primitive_type for int vs num vs str, but i can't see any way to find out about the signedness | 12:16 | |
timotimo: is parse-string doing more joins than it needs to? | |||
timotimo | it used to | ||
it might still do that | 12:17 | ||
it also does concats every now and then | |||
i really must measure this more precisely | |||
MasterDuke | yeah, maybe construct a very small test case that you can calculate how many joins/concats it should only have to do and compare to how many it actually does? | 12:18 | |
timotimo | yeah, it'd just be like "\r\n foo \r\n bar \r\n zap \t yoink \t blubb \t omg" | 12:21 | |
that's the perfect test case right there | |||
MasterDuke | timotimo: somewhat related question. github.com/MoarVM/MoarVM/pull/551 fixes/addresses rt.perl.org/Ticket/Display.html?id=126450, but i'm not sure how to write a test for memory use. could your vmhealth branch help with that? | 12:28 | |
timotimo | hm. bad idea, i think | 12:29 | |
... just write the test so that if the bug is there, it'd consume 64gigs of ram, and if the user's computer crashes during testing, the test failed | |||
(yeah probably don't do that) | 12:30 | ||
MasterDuke | heh, i'd want to reach through the internet and slap someone who merged a test like that | 12:31 | |
timotimo | :D | 12:32 | |
samcv | sounds like a great test | 12:35 | |
fullfills the criteria that when ran, the user will know whether or not the test failed | |||
MasterDuke | i'm not even sure there are many (any?) performance tests in roast, i wonder if it makes more sense to have a separate suite for that | ||
timotimo | yup, nothing wrong with that at all | ||
samcv | MasterDuke, we have the nqp tests | 12:36 | |
and rakudo has some tests but i don't think any moar specific ones? | |||
MasterDuke | doesn't jnthn have a page of moar/rakudo daily benchmarks? | 12:38 | |
timotimo | yup | ||
moarvm.org/measurements/ | |||
we can also put more different kinds of measurement there | |||
MasterDuke | any sort of alerting when values change? | ||
hm, seems to have stopped at 3/8/17 | 12:39 | ||
timotimo | because there's a level of folders in between | ||
no alerting | |||
interesting, it did stop | 12:40 | ||
probably ran out of memory on the server! :P | |||
MasterDuke | are there any plots over time of those benches? | ||
timotimo | don't think so | 12:41 | |
i believe jnthn's daily script nukes the results every day so we don't have histories on a per-day basis | |||
MasterDuke | oh, the *-releases and *-releases-history have some results over time | 12:42 | |
but those seem to have died on 1/31/17 | |||
and those graphs are a little odd, they always have a sharp up or down turn at end, like there's always some bad value | 12:45 | ||
anyway, afk for a while & | |||
Geth | MoarVM: cac2a57c04 | (Stefan Seifert)++ | Configure.pl Do not set use rpath if installing into proper system locations This is a hopefully better attempt than commit f777352ebae29349d1bfa9e6c03c12f32f2ec689. Removing because rpath is prohibited on Fedora and strongly discouraged on openSUSE. It's also not needed if libmoar.so is installed into the default library path. |
12:46 | |
13:37
agentzh joined
15:44
ggoebel joined
15:50
zakharyas joined,
domidumont joined
15:59
IRCFrEAK joined
16:20
IRCFrEAK joined
17:38
agentzh joined
17:54
agentzh joined
18:36
agentzh joined
|
|||
Geth | MoarVM: MasterDuke17++ created pull request #557: Correctly detect+handle overflow in mp_get_int64 |
19:18 | |
MasterDuke | nine: my PR ^^^ was inspired by/based off of the attempted patch you gave me a little while ago, how does it look to you? | 19:29 | |
19:45
agentzh joined
20:01
zakharyas joined
20:10
spebern joined
|
|||
nine | MasterDuke: looks good to me! I wonder how it does on the performance front? | 21:24 | |
MasterDuke | nine: slightly faster. `for ^10_000_000 { my uint64 $a = 2**64-1; my int64 $b = 2**62; my int64 $c = -2**63 }` took an average of 13.4s before, 13.1s after | 23:52 |