Using Multiple images (and threads)

[Derek Magee]

Thanks guys, the mutex stuff was written at 6am to try an work out if it 
was an openslide bug, or just me being stupid [I hadn't spotted the non 
thread safety of openslide-close in the docs until half way through, so 
was trying to work it out for myself]. Mea culpa, a bit crap. I'm 
actually using a "proper" threads library (part of Poco pocoproject.org 
, check it out, I really like it), but hadn't worked out how to do a 
read-write mutex using Poco (a quick google later suggests Poco has a 
class RWLock so I'll give that a go. I'll let you know if I have an 
issues (putting a "proper" mutex round everything seemed to cure the 
crash - hard to tell for sure as it was a rare event - but made 
everything REALLY slow, so it probably is my dodgy mutex code at fault).

I'd suspected a tile cache, or something similar!

Thanks again

D.

On 18/07/2014 03:46, Benjamin Gilbert wrote:
> On 07/17/2014 05:12 AM, Derek Magee wrote:
>> The solution I’ve been using is to close the images, however
>> openslide_close as documented is not thread safe, so I wrap everything
>> up with mutexes:
>>
>> if(!check_openslide()) return false ;
>>
>>       m_doing_openslide_operation=true ;
>>
>> // Some thread safe openslide operation
>>
>>       m_doing_openslide_operation=false ;
> As John said, you have a huge mess here.  A thread can clear
> m_doing_openslide_operation while another thread is still executing in
> OpenSlide, and you don't wait for m_doing_openslide_operation to be
> cleared before starting a close().  More generally, you don't atomically
> test-and-set on your lock paths, and you don't have the memory barriers
> necessary to propagate updated values to other CPUs.  Using Sleep()
> rather than notifying waiters can't help your performance, either.  You
> should really use the standard locking primitives for this kind of
> thing.  (The thing you're trying to implement is called a reader-writer
> lock.)
>
>> ERROR:src/openslide-decode-tiff.c:551:_openslide_tiffcache_destroy:
>> assertion failed: (tc->outstanding == 0)
> This means you're calling openslide_close() on a handle that's still
> being used in another thread.
>
> The proper approach will depend on your use case.
>
> - The reason closing and reopening the slide handle reduces memory use
> is that it frees the handle's tile cache.  If you are continually
> reading from each openslide_t (e.g. for batch processing of the entire
> slide), this can be counterproductive, because the tile cache is there
> to save you CPU time.  In this case, it'd be better to reduce your
> thread count.
>
> - If you're only reading an occasional tile from each slide (e.g. your
> program is a tile server), it'd be better to keep a cache of
> recently-used openslide_t handles, rather than maintaining an
> openslide_t for each slide.  For example:
>
> ===
> // Not tested or even compiled.  This started out as a "simple example"
> // and then turned into a lot of code.  Sigh.
>
> #define MAX_UNUSED_HANDLES 16
>
> struct cache {
>     GMutex *lock;
>     GHashTable *entries;  // path -> cache_value *
>     GQueue *freeable;
> }
>
> struct cache_value {
>     openslide_t *osr;
>     char *path;
>     int users;
> }
>
> struct cache *cache_init(void) {
>     struct cache *cache = g_slice_new0(struct cache);
>     cache->lock = g_mutex_new();
>     cache->entries = g_hash_table_new(g_str_hash, g_str_equal);
>     cache->freeable = g_queue_new();
>     return cache;
> }
>
> // cache lock must be held
> static void _cache_free_one_entry(struct cache *cache) {
>     struct cache_value *value = g_queue_pop_head(cache->freeable)
>     g_assert(value && value->users == 0);
>     g_hash_table_remove(cache->entries, value->path);
>     g_free(value->path);
>     openslide_close(value->osr);
>     g_slice_free(struct cache_value, value);
> }
>
> void cache_destroy(struct cache *cache) {
>     g_mutex_lock(cache->lock);
>     struct cache_value *value;
>     while (!g_queue_is_empty(cache->freeable)) {
>       _cache_free_one_entry(cache);
>     }
>     // no cache entries should still be in use
>     g_assert(g_hash_table_size(cache->entries) == 0);
>     g_mutex_unlock(cache->lock);
>     g_hash_table_destroy(cache->entries);
>     g_queue_free(cache->freeable);
>     g_mutex_free(cache->lock);
>     g_slice_free(struct cache, cache);
> }
>
> openslide_t *cache_get(struct cache *cache, const char *path) {
>     g_mutex_lock(cache->lock);
>     struct cache_value *value = g_hash_table_lookup(cache->entries, path);
>     if (!value) {
>       value = g_slice_new0(struct cache_value);
>       value->path = g_strdup(path);
>       value->osr = openslide_open(path);
>       if (!value->osr || openslide_get_error(value->osr)) {
>          abort();  // don't actually do this
>       }
>       g_hash_table_insert(cache->entries, value->path, value);
>     } else if (value->users == 0) {
>       // We use the O(n) solution for clarity.  O(1) is possible by
>       // tracking the list link in struct cache_value and using
>       // g_queue_delete_link().
>       g_queue_remove(cache->freeable, value);
>     }
>     value->users++;
>     openslide_t *osr = value->osr;
>     g_mutex_unlock(cache->lock);
>     return osr;
> }
>
> void cache_put(struct cache *cache, const char *path, openslide_t *osr) {
>     g_mutex_lock(cache->lock);
>     struct cache_value *value = g_hash_table_lookup(cache->entries, path);
>     g_assert(value && value->users && value->osr == osr);
>     if (--value->users == 0) {
>       g_queue_push_tail(cache->freeable, value);
>       if (g_queue_get_length(cache->freeable) > MAX_UNUSED_HANDLES) {
>         _cache_free_one_entry(cache);
>       }
>     }
>     g_mutex_unlock(cache->lock);
> }
>
> void do_some_operation(struct cache *cache, const char *path) {
>     openslide_t *osr = cache_get(cache, path);
>     openslide_read_region(osr, ...);
>     cache_put(cache, path, osr);
> }
> ===
>
> --Benjamin Gilbert
>
> _______________________________________________
> openslide-users mailing list
> openslide-users at lists.andrew.cmu.edu
> https://lists.andrew.cmu.edu/mailman/listinfo/openslide-users