| | <<<Back 1 day (to 2016/09/16) | 20160917 |
| sebras | Robin_Watts: commit on sebras/master that fixes the issues you and fred talked about yesterday. since to_*() have corresponding to_*_safe() I decided that having from_*() have corresponding from_*_safe() would be good enough. | 06:18.59 |
| Robin_Watts | sebras: will look in a bit, ta. | 08:01.19 |
| sebras | Robin_Watts: ok. I sensed a bit of urgency due to the looming meeting so I fixed it ASAP. | 08:10.55 |
| Robin_Watts | sebras: I think the urgency has gone now that henry has something he's happy with. | 10:12.45 |
| | The commit looks OK to me. Personally, I would have left the returns as they were. | 10:13.18 |
| | i.e. if (!ctx || !map) return; | 10:13.29 |
| | The new formulation means that in Document_finalize Memento_fin() will be called on both destroy and finalize. | 10:14.04 |
| | sebras: 3 commits on robin/master. | 10:18.19 |
| sebras | Robin_Watts: is calling Memento_fin() several times a problem? | 10:20.43 |
| Robin_Watts | No, but it might be unexpected. | 10:20.55 |
| sebras | and our drop-function are supposed to handle null. | 10:21.00 |
| Robin_Watts | indeed. It's not a show stopper. | 10:21.22 |
| sebras | Robin_Watts: right, forgot about the exception messages, sorry. | 10:22.08 |
| Robin_Watts | I had the changes locally already, seems silly for both of us to do it. | 10:22.31 |
| sebras | Robin_Watts: hrm... I need to git grep in platform/ when doing changes, not in platform/java... I know this! why do I keep being silly? :-( | 10:23.12 |
| | Robin_Watts: re: OPEN_MAX... shouldn't that be accessible through limits.h? | 10:25.56 |
| Robin_Watts | it should, but I'm not 100% sure all platforms that gs compiles on have limits.h | 10:26.34 |
| sebras | Robin_Watts: ah, I just noticed the FIXME because you added the ifndef. | 10:27.02 |
| | Robin_Watts: and limits.h ought to be the portable way, right? | 10:27.19 |
| Robin_Watts | yes. | 10:27.42 |
| sebras | Robin_Watts: or programmatically sysconf(_SC_OPEN_MAX) | 10:28.01 |
| | Robin_Watts: in the memento patch, in do_realloc(), why do you ifdef out just the contents of the if-statement instead of the entire if-statement? | 10:32.07 |
| | newsize ought to be used by the assignment immediately following the if-statement anyhow, and the block is empty. | 10:32.31 |
| Robin_Watts | cos I'm dumb :) | 10:32.56 |
| | will fi. | 10:32.58 |
| | fix | 10:33.00 |
| | new version online. | 10:34.50 |
| sebras | Robin_Watts: LGTM. | 10:36.08 |
| Robin_Watts | Thanks. | 10:36.24 |
| sebras | Robin_Watts: what to do about memento_fin()? | 10:36.48 |
| | if (doc) memento_fin()? | 10:36.53 |
| | that ought to work fine. | 10:37.14 |
| Robin_Watts | sebras: I personally would much prefer it if the "cleanup block" (i.e. the latter half of the functions after the initial NULL checks) only ran once. | 10:37.33 |
| | It just feels cleaner. | 10:37.38 |
| | but it's no big deal. | 10:37.45 |
| | In general, I'm in favour of quick exits from code, cos it's easier for human brains to cope with I reckon. | 10:38.33 |
| | and it avoids right hand drift. | 10:38.40 |
| sebras | Robin_Watts: I generally agree with early exit. | 10:40.12 |
| | but I also agree with less characters on the screen means easier to reason about. | 10:40.33 |
| Robin_Watts | http://pastebin.com/KfihdarK | 10:41.03 |
| sebras | and actually using language features/library/framework/whatever to your advantage. | 10:41.15 |
| | Robin_Watts: yeah, I got you the first time. :) | 10:41.38 |
| Robin_Watts | yeah, but I'd started to type :) | 10:41.46 |
| sebras | I'm already updating the commit, calm down! :-D | 10:41.50 |
| | Robin_Watts: new version on sebras/master | 10:44.38 |
| Robin_Watts | Lovely. lgtm. | 10:46.30 |
| | Thanks for indulging me :) | 10:46.37 |
| sebras | Robin_Watts: hey, you fix whatever silly small things I find, so it goes both ways. | 10:47.15 |
| | Robin_Watts: it's all needed to get to know each other as programmers and lessen the review annoyances. | 10:47.41 |
| | Robin_Watts: in fz_load_tiff_subimage() | 11:14.06 |
| | why do we set alpha to 1 to fz_new_pixmap()? | 11:14.09 |
| | Robin_Watts: unconditionally I mean. there must be a viable way of detecting if the tiff has alpha or not. | 11:14.56 |
| Robin_Watts | I cannot remember details. | 11:20.19 |
| sebras | Robin_Watts: seems like there's a field called extrasamples which is != 0 for pre-multiplied (or not) alpha and 0 if the extra components contain some other type of data. | 11:21.31 |
| | Robin_Watts: patch on sebras/master | 11:21.37 |
| Robin_Watts | sebras: If that clusters out Ok, then lgtm. | 11:43.26 |
| sebras | Robin_Watts: unfortunately now. | 11:43.36 |
| | no. | 11:43.37 |
| | Robin_Watts: now I know why it didn't cluster well. the svg interpreter was broken and has been since its inception! | 13:30.05 |
| | Robin_Watts: two svg patches and one fz_tree patch on sebras/master | 13:30.16 |
| | now it clusters! | 13:30.22 |
| Robin_Watts | sebras: Oh, cool. | 14:35.14 |
| | All lgtm. That solves the svg segv's and errors? Fab. | 14:36.06 |
| sebras | Robin_Watts: it does. | 14:52.37 |
| Robin_Watts | Nice one. I was planning to take a look at that, but you've saved me the trouble. | 14:53.02 |
| sebras | I thought I had messed up in the tiff code somewhere and that the svg-files embedded tiff-files. | 14:53.07 |
| Robin_Watts | I only added SVG files to the cluster last night :) | 14:53.25 |
| sebras | Robin_Watts: oh, this was a known indeterminism? | 14:53.30 |
| Robin_Watts | since last night :) | 14:53.43 |
| sebras | Robin_Watts: right. the error has been there since svg was added though. | 14:54.21 |
| | Robin_Watts: spooky that we see it first now though. | 14:54.29 |
| Robin_Watts | I think it's unusual to see errors until you start testing the code :) | 15:00.13 |
| sebras | Robin_Watts: oh, the .svg files were recently added to the cluster? | 15:05.55 |
| Robin_Watts | last night. | 15:06.07 |
| sebras | Robin_Watts: 3de83b0d7a679c2c91587718c1c273775b99e8dd causes Bug693416.pdf to get stuck in an endless loop in fz_filter_store() | 17:00.30 |
| Robin_Watts | sebras: oh? | 17:10.30 |
| | With what command? | 17:11.47 |
| sebras | build/debug/mutool draw -s t Bug693416.pdf | 17:40.18 |
| | btw, fz_print_store() is utterly broken. | 17:40.41 |
| | I'm fixing that one. | 17:40.44 |
| | the loop might actually not be endless, but veeery long. | 17:41.10 |
| | longer than it used to be. | 17:41.16 |
| Robin_Watts | Presumably you're using the latest version? | 17:43.46 |
| | cos that has changed fz_filter_store a lot. | 17:43.53 |
| sebras | I am. | 17:44.12 |
| | Robin_Watts: I know. the commit I mentioned introduced your key storable thingy. | 17:45.51 |
| | Robin_Watts: it's causing a timeout in the cluster. | 17:46.08 |
| | fz_drop_display_list() used to be more or less instantaneous on that file, but now it takes takes more than 30 seconds (I haven't yet worked out if it is in an eternal loop, I'm starting to think that this is not the case). | 17:50.46 |
| | let me wait it out. | 17:51.26 |
| Robin_Watts | 8 million things in the store. ouch. | 17:52.27 |
| sebras | yeah, it eventually finished after 75 seconds. | 17:53.46 |
| Robin_Watts | Need to ponder on that. Will walk dogs. | 17:56.35 |
| sebras | Robin_Watts: these tings have been in the store before, right? | 17:56.48 |
| | see you. | 17:57.21 |
| | Robin_Watts: cache load 111821 / 262144 from the version prior to the store changes. | 18:11.12 |
| | Robin_Watts: identical in the new version of course. | 18:20.25 |
| | Robin_Watts: now, for every image referenced by the display list that we're dropping we are first determining that only_refs_are_store_key_refs == 1 in fz_drop_key_storable() | 20:05.21 |
| | next fz_drop_image() calls fz_filter_store() | 20:05.31 |
| | which for every image loops through all the images in the store trying to assess if this entry is the image to drop | 20:05.56 |
| | once found it will be removed. | 20:06.07 |
| | so O(n^2) right..? | 20:06.15 |
| | previously we never called fz_filter_store() in this case, so we just got O(n) when removing the display list node image references. | 20:07.15 |
| | do we have to remove all images only referenced by the store on _every_ image removal from the store? | 20:08.20 |
| | Robin_Watts: if we were to only do it on every 100th call the time would definitely be reduced. | 20:08.56 |
| | Robin_Watts: yes. if I comment out fz_filter_store() from fz_drop_image() it takes me 6s to do mutool draw -s t instead of 74s. | 20:09.59 |
| sebras | needs to sleep. | 20:13.55 |
| Robin_Watts | I've been pondering the idea of a reap list - put the ones to be reaped on the list and then reap them... less frequently. | 20:55.49 |
| | doing the reap list at the store level would be nicest, but it's harder. | 20:56.22 |
| | doing it at the image level would be doable, but then it's harder to get it triggered on low memory. | 20:56.45 |
| | needs more thought. | 20:56.49 |
| | Forward 1 day (to 2016/09/18)>>> | |
IRC Logs