GNOME Bugzilla – Bug 342110
Keep rendered page thumbnails
Last modified: 2017-06-05 13:24:38 UTC
Page thumbnails in Evince are always the same size and don't change. Now when I scroll in thumbs section, Evince loses already rendered thumbnails and when I scroll back, it renders new ones (taking some time at 100 % CPU usage). I suggest to keep already rendered thumbs and display them to get rid of waiting "new" ones and wasting CPU time. Other information: Ubuntu Dapper, Gnome 2.14 Evince 0.5.1
I don't know we should change current behavior. Consider document with 1000 pages - it's thumbnails will take too much memory. Probably we should save them to the disk but I'd better like faster thumbnails rendering instead.
I understand. But current behaviour makes thumbnails on older computers unusable. If one thumbnail has up to 100 x 150 px and 4 B/px then one thubnail takes approximately 60 kB and 100 thumbnails take 6 MB. If there would be a limit of 10 or 15 MB limit for the thumbnails, it could make browsing faster and would not take much RAM. Finally, it doesn't matter very much, whether it will be solved by faster rendering or keeping in memory. But it should be solved.
I would be happy to have the thumbnails cached in memory too. Seeing the thumbnails is really improving document browsing, and having to wait for thumbnails to rebuild is really annoying. This feature is available in KPDF, and I find it's useful.
Thumbnails are usually rendered really fast, I don't think it's worth to waste memory caching them. Evince already uses a lot of memory caching rendered pages.
This is a pretty bad bug I would say. It makes Evince very, very broken for several documents I have that are exported slide decks with an image on each page. (A common type of document.) Rendering these pages is much too slow to do on demand. I have a fast computer, too. The number-one task with a document like this is to find a particular page and look at it. But in evince, as you scroll through the pages themselves they keep re-rendering which takes forever, so you can't quickly look for a page; and the thumbnail sidebar would seem to be the solution, except it has the same problem. The thumbnails should be possible to store as simply image data, they should not be using any appreciable amount of memory, no more so than an icon of that size. It would help quite a bit to just toss out the renderings in a timeout of 1 minute or so instead of immediately, so you can scroll around and find something without repeatedly pausing for re-render. Let the virtual memory subsystem deal with memory, within reason; it is not a big deal if evince blows up to a lot of memory temporarily, especially if the memory is in big contiguous chunks of image data, and it avoids "touching" memory except on demand, because then stuff will get paged out. Treat 1000-page documents as a special case somehow, but usability of a 20-page slide deck should not be sacrificed to that corner case. Simple approach could be something like: always cache the last 100 pages rendered, plus the current page, and pre-render the page before and after current page even before the user switches to them. bug #320567 is related and would also help with this.
just store the last 20 pages BUT DO PRE RENDER next 2 pages or so that is more important.
This bug is still present in 3.14.1. With complex and "heavy" PDFs, e.g. of photo books, this is really noticeable even fast systems. http://gimpmagazine.org/
> Consider document with 1000 pages - it's thumbnails will take too much memory. First, thumbnails are small. Expanding sidebar to full screen (1920x1080=2Mpx) I can fit ~100 thumbnails, so a 1000 pages will take < 20Mpx*RGB = 60MB uncompressed. A graphics-rich 1000 page PDF file could itself be order of GB, so if I can view it at all (with a few pages cached), thumbnails would be small change. Second, how do expect me to navigate a 1000-page book without thumbnails?! I'm now reading a 60-page 100MB graphics-rich book and each page takes about 1 second to get rendered, and thumbnails only start rendering after that, with similar speed. As a user, in the worst case I'd rather see thumbnails swapped out than waiting for them to re-render. If you're really concerned about memory, better downsample and/or JPG-compress out-of-view thumbnails than drop them. Havoc Pennington's idea of delaying dropping until a minute has passed would also be a great step forward, allowing fast "navigation" periods, though if it takes a whole minute to render the 60 thumnails after a "reading" period, that would still feel slow.
(In reply to Carlos Garcia Campos from comment #4) > Thumbnails are usually rendered really fast, I don't think it's worth to > waste memory caching them. Evince already uses a lot of memory caching > rendered pages. That's not true in practice. It still takes some time even for simple and small PDF files, and it is often noticeable.
Created attachment 352757 [details] [review] sidebar-thumbnails: keep thumbnails already rendered sidebar-thumbnails: keep thumbnails already rendered Evince renders thumbnails on-the-fly as they get into the scrolling visible area, but at the same time it will remove them as they get out of the visible scrolling area, so when user scrolls back to same position he will notice thumbnails be recreated. In pro of a more icing user experience, let's adopt a mixed approach and keep the thumbnails that the user has already navigated, so when he scrolls back and forth in the same area no thumbnail re-generation will be visible. This also matches behaviour with other pdf readers. https://bugzilla.gnome.org/show_bug.cgi?id=342110
Review of attachment 352757 [details] [review]: Have you measured the difference in memory consumptions with long documents? What we could do, I think, is reducing the area to clear, so instead of clearing non-visible area we keep one "page" after and before the current range. In this case I don't mean a document page but a scrolling area page. ::: shell/ev-sidebar-thumbnails.c @@ +442,3 @@ + if (thumbnail_set) + continue; You would be leaking the job here if it's not NULL. But I think this is essentially making clear_range do nothing, no? If we really want to keep thumbnails always cached, we should just remove clear_ranges entirely I guess. @@ +447,3 @@ g_signal_handlers_disconnect_by_func (job, thumbnail_job_completed_callback, sidebar_thumbnails); ev_job_cancel (EV_JOB (job)); g_object_unref (job); Ah, I see, we cancel ongoing jobs, but keep rendered thumbnails. If this is what we want I would rename this function as cancel_running_jobs or something like that.
(In reply to Carlos Garcia Campos from comment #11) > Have you measured the difference in memory consumptions with long documents? > What we could do, I think, is reducing the area to clear, so instead of > clearing non-visible area we keep one "page" after and before the current > range. In this case I don't mean a document page but a scrolling area page. By default, those thumbnails are 100px by 141px. With 32 Bit colors (RGBA) it is 56kB per thumbnail. Anything less than 1000 pages should not be a problem. On HiDPI devices, the thumnail size might increase by factor 4, which still is no problem for having <100 thumbnails in RAM at once. I'd rather not delete thumbnails so aggressively. On a 40-pages document evince could easily keep all thumbnails.
(In reply to Carlos Garcia Campos from comment #11) > Review of attachment 352757 [details] [review] [review]: > > Have you measured the difference in memory consumptions with long documents? Not really, just assuming current computers are not scarce on ram, and user is to small degree savvy on how many and big applications he has opened (ram usage), I mean a normal user if he has a web browser with 50 tabs open and a couple more misc programs opened, and he notices his computer is getting slow he knows he has to close so many tabs or close some video that is playing, so same way if user has opened a 5000 page PDF (which is non common pdf) and he notices computer getting slow he can assume the reason and decide upon it (close the PDF or other big apps he has opened). > What we could do, I think, is reducing the area to clear, so instead of > clearing non-visible area we keep one "page" after and before the current > range. In this case I don't mean a document page but a scrolling area page. The problem is that when current code, you're constantly adding and clearing the same ranges (not exactly the same but randomly) in the background for the intermediate pages when you do a fast scroll, for fast scroll I mean when user is eg. at the first pages of pdf, and what to check some pages at the end of pdf and for that the user grabs the scroll and does a quick pull to reach the end, what happens in the background (caused by the "adjustment_value" handler be fired so quickly) is that all pages in-between are all randomly added to batches of 'add' and 'clear', this leaves you with thumbnails set randomly (those that win the race to 'clear') across all the range you fast scrolled. > > You would be leaking the job here if it's not NULL. But I think this is > essentially making clear_range do nothing, no? If we really want to keep > thumbnails always cached, we should just remove clear_ranges entirely I > guess. > > > Ah, I see, we cancel ongoing jobs, but keep rendered thumbnails. If this is > what we want I would rename this function as cancel_running_jobs or > something like that. At first I was about to remove 'clear_ranges' entirely, then I realised the problem of fast scroll I described above, so I had to keep 'clear_ranges' so it can cancel a lot of the 'add' batches that are automatically added when you do a fast scroll. Thinking of solutions, one would be (on top of my current patch) to add some code to "adjustment_value" handler so it can detect a fast scroll (check timestamp from previous call) and not act upon it. Then we could add your suggestion of making a bigger 'clear' and 'add' range. And then we could add a MAX_THUMBNAILS_SET=200 limit, meaning that when the number of thumbnails set reaches this, we revert back to dynamically adding/removing thumbnails as they go in/out scroll range (this is current behaviour prior to my patch).
Review of attachment 352757 [details] [review]: Ok, let's commit this for now then. But please, rename clear_ranges and ensure we don't leak jobs, either adding an assert if thumbnail_set can't be TRUE when job != NULL, or checking thumbnail_set after cancelling the job.
(In reply to Carlos Garcia Campos from comment #14) > Review of attachment 352757 [details] [review] [review]: > > Ok, let's commit this for now then. But please, rename clear_ranges and > ensure we don't leak jobs, either adding an assert if thumbnail_set can't be > TRUE when job != NULL, or checking thumbnail_set after cancelling the job. Done in commit 121e4d99a59c0c0586a4962f18423139e568f9f7 thank you! Carlos, I think your suggestion of: keep one "scrolling area page" after and before the current range would improve even more the user experience, because currently when I scroll the thumbnail sidebar even at a slow pace, I can see thumbnails be created as I scroll into them. So I will try to cook a patch for that and attach here.
Created attachment 353114 [details] [review] sidebar-thumbnails: preload one extra visible range while scrolling sidebar-thumbnails: preload one extra visible range while scrolling Preload both before and after current visible scrolling range, the same amount of thumbnails in it, to help prevent thumbnail creation happening in the user's sight. https://bugzilla.gnome.org/show_bug.cgi?id=342110#c15
Review of attachment 353114 [details] [review]: Cool! ::: shell/ev-sidebar-thumbnails.c @@ +621,3 @@ + n_pages_in_visible_range = (end_page - start_page) + 1; + + start_page = MAX (0, start_page - n_pages_in_visible_range); Please remove the extra space before - @@ +622,3 @@ + + start_page = MAX (0, start_page - n_pages_in_visible_range); + end_page = MIN (priv->n_pages - 1, end_page + n_pages_in_visible_range); I'm not a fan of this king of line ups, I would remove the extra spaces before the = @@ +624,3 @@ + end_page = MIN (priv->n_pages - 1, end_page + n_pages_in_visible_range); + + update_visible_range (sidebar_thumbnails, start_page, end_page); This name is not accurate anymore, since it's not receiving the visible range. I think we could move the preloaded range calculation to update_visible_range, though. Then it receives the visible range and preloads a bigger range, for example.
(In reply to Carlos Garcia Campos from comment #17) > Review of attachment 353114 [details] [review] [review]: > > @@ +624,3 @@ > + end_page = MIN (priv->n_pages - 1, end_page + n_pages_in_visible_range); > + > + update_visible_range (sidebar_thumbnails, start_page, end_page); > > This name is not accurate anymore, since it's not receiving the visible > range. I think we could move the preloaded range calculation to > update_visible_range, though. Then it receives the visible range and > preloads a bigger range, for example. Done in commit 1fc8c68c53daa89 Thanks!