tingbot / tingbot-python

🔩 Python APIs to write apps for Tingbot
Other
18 stars 9 forks source link

Add caching for images #26

Closed furbrain closed 8 years ago

furbrain commented 8 years ago

This pull request implements an ImageCache - this can retrieve files from the web or local storage, but will not hold more than approx 2Mb in memory at any one time and will drop the oldest images if this limit will be over-reached. Images retrieved from the web will be "re-retrieved" if they have exceeded their cache times as per http protocols. Images from local storage will be reloaded if they have changed on-disk,

This PR addresses the second part of issue #13 - intelligent caching

joerick commented 8 years ago

Taking a look at this now 😎

joerick commented 8 years ago

I wanted to know how the cache would cope with a really large image.

screen.image('gif_large.gif', scale=0.5)

(The large GIF I used for this - 12MB on disk, 42MB in memory as reported by get_memory_usage)

I got the following error:

Traceback (most recent call last):
  File "/Users/joerick/Work/Tingbot/6 tingbot-lib-dev/tingbot/run_loop.py", line 65, in run
    next_timer.action()
  File "gif_large_test.py", line 6, in loop
    screen.image('gif_large.gif', scale=0.5)
  File "/Users/joerick/Work/Tingbot/6 tingbot-lib-dev/tingbot/graphics.py", line 270, in image
    super(Screen, self).image(*args, **kwargs)
  File "/Users/joerick/Work/Tingbot/6 tingbot-lib-dev/tingbot/graphics.py", line 212, in image
    image = image_cache.get_image(image)
  File "/Users/joerick/Work/Tingbot/6 tingbot-lib-dev/tingbot/cache.py", line 143, in get_image
    return self.images[location].get_image()
KeyError: 'gif_large.gif'

I think it was deleted out of the cache before the function returned.

joerick commented 8 years ago

I also did a little bit of cleanup as I was testing - pull from branch pull-26 before making more changes please. Thanks!

furbrain commented 8 years ago

Ok, have pulled in data from pull-26, fixed bug with GIF images and with large files and added tests for this

furbrain commented 8 years ago

One question - do we need to make the ImageCache thread safe? I'm trying to think of a situation where it would break...

furbrain commented 8 years ago

Got it - two threads calling get_image and exceeding the size of the cache, triggers two competing loops clearing out the cache. I'll lock the get_image and del_image functions

joerick commented 8 years ago

Merged 👍 cheers @furbrain!