| <<<Back 1 day (to 2018/01/09) | 20180110 |
malc_ | tor8, sebras: i was expecting b840e169b486850867f71faf0cdd30e329170f30 to largely address tiling "slowdown", but that doesn't appear to be the case, for instance: the speed of tiled rendering of https://upload.wikimedia.org/wikipedia/commons/5/56/Peasant_Family_of_Ramallah_1900-1910.jpg is abysmal (compared to the case when the jpeg is first converted to png and that is rendered/tiled) (also amusingly png is almost 10x smaller than | 16:59.24 |
| jpeg, go figure) | 16:59.24 |
sebras | malc_: with the world map PNG that we tested last time, this now actually caches the full image, even though you just requested a subarea of it. so the file is not fully decoded once per tile. | 17:34.14 |
| malc_: still remaining are the locking issues that you mentioned. | 17:34.29 |
| malc_: and no one has looked into the performance issue you mentioned above yet. | 17:35.10 |
malc_ | sebras: okay, i just assumed that the aforementioned commit would largely "solve" the rerendering of whole whatever issue | 17:36.26 |
sebras | Robin_Watts: I think fz_lock_debug_lock() is buggy, and perhaps fz_lock_debug_unlock() too. | 18:06.26 |
Robin_Watts | sebras: Howso? | 18:06.41 |
sebras | Robin_Watts: consider the case of two threads competing for the same lock. | 18:06.55 |
Robin_Watts | The debug lock stuff is thread unsafe, isn't it? | 18:07.08 |
sebras | it is. | 18:07.15 |
Robin_Watts | That's known. | 18:07.20 |
| It's only supposed to be used in single threaded programs to check that you never take the locks in the wrong order. | 18:07.42 |
| To make it thread safe would probably require another lock, or something :) | 18:08.07 |
sebras | Robin_Watts: ok, but then we shouldn't be printing things like "Attempt to take lock %d when held already!", right? | 18:08.08 |
| it would! :) | 18:08.14 |
Robin_Watts | sebras: If you run a multithreaded app using MuPDF build with lock debugging turned on, you are asking for trouble. | 18:08.56 |
sebras | Robin_Watts: perhaps we should still print it to catch multiple calls to fz_lock() for the same lock, since we allow for non-recursive locks. | 18:08.57 |
| Robin_Watts: that's exactly what malc_ (and hence I) is doing. | 18:09.15 |
| Robin_Watts: we asked for trouble, and we got it. | 18:09.24 |
Robin_Watts | sebras: Don't do it. | 18:09.26 |
| There be dragons, and they will eat you. | 18:09.35 |
| You can't trust anything it says when it's used like that. | 18:09.47 |
sebras | Robin_Watts: though these lock debugging functions are automagically turned on for build=debug, no? | 18:10.09 |
Robin_Watts | And having different threads attempting to call fz_lock at the same time for the same lock, is the whole point of locks. | 18:10.19 |
| eek. Is it? | 18:10.22 |
sebras | mupdf/source/fitz/fitz-imp.h 95 | 18:10.33 |
| perhaps I'm negating something wrong when I'm thinking, but I think they will be enabled. | 18:11.02 |
Robin_Watts | sebras: oops. | 18:11.12 |
| OK, so we need... | 18:11.15 |
sebras | Robin_Watts: one option is to simply add another lock for lock debugging of course. | 18:12.53 |
| Robin_Watts: and to make the lock debugging functions be thread safe. | 18:13.12 |
Robin_Watts | if (ctx->locks.lock == fz_lock_default) return; | 18:13.40 |
| in the lock debugging stuff. | 18:13.54 |
| That would be simplest. | 18:13.59 |
| Or we could add a new lock. | 18:14.06 |
| Dealers choice. | 18:14.12 |
sebras | Robin_Watts: let me try both approaches. | 18:14.27 |
| Robin_Watts: so that would then be FZ_LOCK_LOCK_DEBUG right next to FZ_ALLOC_ALOC and FZ_ALLOC_FREETYPE. :) | 18:14.56 |
Robin_Watts | something like that, yes. | 18:15.34 |
| Forward 1 day (to 2018/01/11)>>> | |