| <<<Back 1 day (to 2020/07/22) | Fwd 1 day (to 2020/07/24)>>> | 20200723 |
apollo13 | Robin_Watts: yeah, that is certainly the case for some warnings. But still, the warnings would go away if you initialized those to zero or so | 09:05.15 |
| Robin_Watts: generally I have the feeling that clang is a bit smarter anyways | 09:06.57 |
avih | ator: weekly ping mujs proportional GC threshold. | 09:16.14 |
apollo13 | soo, another question regarding mupdf & java threading | 09:23.26 |
| I am still on the search for a few segfaults | 09:23.34 |
| can I share a Document between threads? | 09:23.51 |
| I have a synchronized around the usage, but is it a valid usage? | 09:24.09 |
Robin_Watts | apollo13: In general, with MuPDF we have 3 rules for threading. | 09:30.13 |
| No simultaneous calls to MuPDF in different threads are allowed to use the same context. | 09:31.32 |
| No simultaneous calls to MuPDF in different threads are allowed to use the same document. | 09:31.40 |
| No simultaneous calls to MuPDF in different threads are allowed to use the same device. | 09:31.47 |
| The first one is handled for you by the java wrapper. | 09:32.04 |
| but the other two are probably still down to you. | 09:32.40 |
apollo13 | well I have the others under control. what I don't know is if I have a Document object in Java can I call loadPage on it from two different threads (it is ensured that they run after each other) | 09:33.44 |
Robin_Watts | We probably ought to be synchronising on the use of a Document. | 09:34.00 |
| that *should* be fine. | 09:34.14 |
sebras | Robin_Watts: n.b. there is no locking in the JNI layer at all. | 09:34.44 |
Robin_Watts | As long as they don't run at the same time, you should be fine. | 09:34.44 |
sebras | Robin_Watts: all locking happens at the C-level. | 09:34.51 |
Robin_Watts | sebras: yeah, just pondering whether synchronizing everything on Document might solve potential problems. | 09:35.39 |
apollo13 | mhm I am still wondering if I have GC issues after the latest fixes from Robin_Watts. Sadly the current issues do not have a reproducer yet | 09:36.19 |
sebras | Robin_Watts: well, you have other objects relating to the document such as PDFPage. you'd need to synchronize on the document from the PDFPage-accessors. | 09:36.40 |
Robin_Watts | yeah. | 09:37.02 |
apollo13 | can you recommend a good way to debug segfaults? | 09:37.14 |
Robin_Watts | apollo13: within java? no, sorry. | 09:37.26 |
apollo13 | My next approach is to link against libsegfault to get better traces (hopefully) | 09:37.29 |
sebras | apollo13: make sure that the MuPDF library you use from within java is built using debug, or else you'll not get any useful backtraces. | 09:38.12 |
| apollo13: are you using java desktop or android? | 09:38.30 |
apollo13 | desktop | 09:38.35 |
| inside a webapp | 09:38.39 |
sebras | apollo13: do you have the source somewhere? | 09:39.02 |
apollo13 | sebras: this is the current https://gist.github.com/apollo13/66c3eca2ae6f860aa8cdfeb97e904965 rendering implementation | 09:43.32 |
| we have synchronized around renderPage & getPageInfo, so I still suspect other GC issues | 09:44.12 |
| we also tried fixing issues by calling .destroy initially but that did lead to double free issues | 09:44.39 |
| although I think you commited a few things on master for that | 09:45.00 |
| ie http://git.ghostscript.com/?p=mupdf.git;a=commitdiff;h=a5a668fde9d922cd01ab23e6dd13c869f924e2a4 | 09:45.21 |
ator | avih: Applying: gc: use proportional instead of fixed threshold | 10:00.44 |
| error: patch failed: jsgc.c:135 | 10:00.44 |
| error: jsgc.c: patch does not apply | 10:00.44 |
| Patch failed at 0001 gc: use proportional instead of fixed threshold | 10:00.44 |
avih | ator: i'm assuming you applied this https://0x0.st/iyVY.txt but few minutes later i posted https://0x0.st/iyV7.txt (after rebase) which replaces it and should apply | 10:28.37 |
| let me know if it still doesn't apply. it's been a while since i touched it. | 10:29.38 |
ator | avih: I am trying to 'git am iyV7.txt' and get that message | 10:31.46 |
avih | ator: hmm.. maybe there were changes after i posted it. i'll repost a new pair of patches in few mins at most. | 10:32.27 |
ator | trying to apply it to origin/master | 10:32.28 |
avih | ator: the two patches on top of master (ed44bc0) https://0x0.st/iwQS.txt | 10:34.16 |
| ed33bc0 * | 10:34.26 |
ator | avih: thanks. those applied cleanly. | 10:34.57 |
avih | thx. sorry about the conflict. | 10:35.16 |
| ator: the v8 bench test splay.js really sheds important light about the impact of gc frequency. counting the properties while keeping the same 10K threshold (first commit) reduces the score by a factor of 5, and then making gc proportional for 80/20 increases its score by a factor of 50. | 10:39.59 |
ator | avih: yeah. this all looks fine. have you experimented with other factors? | 10:40.46 |
avih | no | 10:40.54 |
ator | or how did you arrive at 5.0 ? | 10:41.00 |
avih | out of nowhere, really | 10:41.29 |
| fwiw observing luajit memory usage pattern, suggests a factor of 2-3 with similar approach as these patches | 10:42.15 |
| ator: ^ | 10:42.19 |
| ator: the main consideration was that i wanted gc to mostly do actual work, which means factor of 2 or more. | 10:43.20 |
| so with the 80/20 paretto rule, i made it do 80% work | 10:43.41 |
| (work == objects freed) | 10:44.07 |
ator | well, it's certainly no worse than what we had :) | 10:45.58 |
avih | the reasoning? or behavior? | 10:46.14 |
ator | both :) | 10:46.21 |
avih | :) | 10:46.26 |
ator | only other "improvement" I can think of would be to allow setting a memory limit in terms of KB, and running gc when that is hit | 10:46.59 |
| but then you'd have very inflexible and bad performance if you're running up against the limit | 10:47.17 |
avih | yeah. i thought about doing gc on OOM, but if implemented trivially then i'm pretty sure it will lead t use after free | 10:47.42 |
| well, if the user specifies a limit, i assume they absolutely don't want to use more, so obeying triumphs i think | 10:48.58 |
| ator: however, implementing a limit - same as gc on OOM - is not easy, because any temporarily allocated thing which was not already linked into one of the lists could get freed. so if you a = js_malloc(...); b=js_malloc(...); <use a and b for a new object>, then if the limit is hit on b then a will be lost because it's not linked. | 10:52.22 |
| and if you only test it when gc can be triggered, then you can easily overshoot the limit | 10:52.45 |
ator | avih: yeah. it'd have to be a "soft" limit | 10:52.46 |
| anyway, this looks fine. sorry for taking so long to get around to looking at it. | 10:53.06 |
avih | no worries. thanks for looking at it. | 10:53.18 |
| ator: however, memory reporting function could be useful. in mpv it's possible to debug scripts and observe their memory usage etc, so if you can have some API to retrieve usage (overall, and if you want, also only "linked" allocations), it could be nice. | 10:55.06 |
| this can also be implemented using a custom allocator, but then it would have overhead, complexity, and mujs already know how much it allocates anyway (however, not how much it frees, which, especially for lstrings, could be an issue) | 10:56.39 |
| ator: do you want me to implement soft limit? my idea is that by default there's no soft limit, but you can change it at any time with an API call. this means mujs will need to account for all allocations and frees. and then at a new commit i can add mem usage retrieval API. should i do that? | 10:59.31 |
ator | avih: nah, I'd just wait until we get someone actually asking for it :) | 11:00.24 |
avih | the only case which needs consideration is js_push_lstring, because the size can be bigger than strlen(v)+1 | 11:00.33 |
| (but not smaller, iirc it does add terminating null) | 11:01.05 |
| sure. | 11:01.33 |
| ator: if you have scripts which you use for testing, you can make the gc report unconditional, and observe the first %-freed value, and see how close it is to 80% even with dynamic usage patterns. | 11:11.25 |
| i was expecting much less accuracy, and was pleasantly surprised of how well it works | 11:12.09 |
| also, i was pleasantly surprised of how quickly the threshold stabilizes at any kind of script really. it starts as 0, and can reach millions within very few gc cycles. | 11:15.42 |
| (like, 5 few) | 11:16.07 |
ator | avih: only if you have scripts that generate actual garbage :) | 11:50.43 |
avih | my scripts are perfect. mujs generates the garbage :) | 11:51.16 |
| ator: how useful and/or complex do you think it would be to use reference counting instead of gc? | 11:53.00 |
| i think complexity would only be for detecting cycles, right? | 11:54.10 |
| "only" :) | 11:55.20 |
| hmm.. i _think_ you changed "XX %" to "XX%" ? iirc the rule is that the unit is always separated by space from the value. | 12:02.30 |
| https://physics.nist.gov/cuu/Units/checklist.html (#15) | 12:05.25 |
paulgardiner | I've updated the SDK repo and linked into the SO one, so potentially it can be published and worked on now. At the moment the only copy is in my casper repos directory. I'm thinking there's probably a better place. Any thoughts. | 12:07.41 |
| sebras: also there are the issues you raised. I'm not sure if you were offering ta address them, but if so that's possible now (although perhaps not pushing back to the repo where it currently sits). | 12:09.02 |
| <<<Back 1 day (to 2020/07/22) | Forward 1 day (to 2020/07/24)>>> | |