Optimize the cache size to avoid unnecessary re-rendering#56
Conversation
At higher zoom levels, the hard-coded limit of 200MB may be too small to hold all the displayed tiles, forcing them to be re-rendered each time the widget is repainted. This in turn can cause flickering as the tiles are erased, re-rendered, and repainted. This is especially noticeable when scrolling over a page break; rendering each page purges the other's tiles from cache, causing both to be re-rendered at each increment. This patch scales the cache size dynamically with the page size to prevent re-rendering in this scenario while still limiting memory usage as much as possible.
These were allowing images to stay in cache longer than needed when the cache was at or near its maximum size, which was wasting memory at higher zoom levels.
This is a fairly self-explanatory memory optimization.
This is mainly a workaround for large tiles surviving purge() when reducing the zoom level, but honestly it seems like a reasonable thing to do anyway in its own right.
|
@bmjcode I'm not able to review your code. Let me know if you want this PR to be included in the 1.0.5 release or you prefer waiting. |
| # adjust the cache size to fit at least two pages' visible tiles | ||
| # to avoid re-rendering either page when both are displayed | ||
| oldsize = self.maxsize | ||
| self.maxsize = max(type(self).maxsize, 2 * tileBytes) |
There was a problem hiding this comment.
Previously, the cache size was fixed at 200MB, which was too small for the tile images at higher zoom levels. This code dynamically adjusts the size to the larger of 200MB or two pages' visible tiles. We do two pages because otherwise each would purge the other from cache when scrolling over a page break, forcing frequent re-renders (you can see this now scrolling over a page break at 800% zoom).
| self.currentsize += e.bcount | ||
|
|
||
| if not purgeneeded: | ||
| def purge(self, reservedsize=0): |
There was a problem hiding this comment.
This was previously part of addtile() and done after the new tile was added. I made it a separate method so it could be called before, which is more efficient.
| for time, bcount, group, ident, key, tile in entries: | ||
| currentsize += bcount | ||
| if currentsize > self.maxsize: | ||
| if currentsize >= limit: |
There was a problem hiding this comment.
Changing to >= fixes a bug in the original code where an extra image could be added when the cache was exactly at its size limit.
| pending job. | ||
|
|
||
| """ | ||
| self.cache.prepare(tiles) |
There was a problem hiding this comment.
This calls our new code to adjust the maximum cache size and purge old tiles just before creating the jobs to render new ones. Previously, the maximum cache size was fixed, and old tiles were purged after rendering and caching new ones.
|
@fedelibre I'd like to get this and #58 into the new release because they are both significant optimizations. That said, I also want you to be comfortable with the changes before you approve them. I added some more detailed explanations on here in case that helps. |
|
Done. There's one more big optimization in #58 I'd like to include. I didn't originally expect to have it done for this release, but it ended up being quicker to implement than I'd anticipated. It also made enough of an improvement that I thought it would be silly to delay it any further. |
2 participants
At higher zoom levels, the hard-coded limit of 200MB may be too small to hold all the displayed tiles, forcing them to be re-rendered each time the widget is repainted. This in turn can cause flickering as the tiles are erased, re-rendered, and repainted. This is especially noticeable when scrolling over a page break; rendering each page purges the other's tiles from cache, causing both to be re-rendered at each increment.
This PR scales the cache size dynamically with the page size to prevent re-rendering in this scenario while still limiting memory usage as much as possible. It also makes several improvements to the cache logic, such as purging old tiles before rather than after rendering new ones, to further limit memory usage even with a larger cache size.