Rob,
Thanks for diving in this, also been on holiday.
I agree with your understanding:
> There are two data structures in the mapcache:
> 1. An actual hash containing keys against map tiles - this makes sense.
> 2. A circular linked list queue (using own implementation - I don't think
> there is a glib support for ringbuffers)
>
> I don't understand the point of the second one. The queue seems to be
> equivalent of the hash cache and used just to compare the keys stored in
> deciding which ones can be removed.
> After further thought, I now believe the second data structure has
use of a LIFO queue, so that when the maximum cache size is reached it
removes the oldest tiles (in terms of being added to the map cache).
I still not get though why some kind of circular list was used, that
only complicates things.
If the need indeed is to remove the oldest tiles a singly-linked list
(g_slist) should suffice and make things clearer.
> I have committed a basic fix for this which checks tmp before trying
to use it.
I did a git pull and tried to compile things but got an compile error:
$ gcc -std=gnu99 -DHAVE_CONFIG_H -I.
-DVIKING_DATADIR=\""/usr/local/share/viking"\"
-DVIKING_SYSCONFDIR=\""/usr/local/etc/viking"\" -Wall -g -D_GNU_SOURCE
-pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include
-I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo
-I/usr/include/pixman-1 -I/usr/include/libdrm -I/usr/include/libpng12
-I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libpng16
-I/usr/include/pango-1.0 -I/usr/include/harfbuzz
-I/usr/include/pango-1.0 -I/usr/include/freetype2
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -g -O2 -MT
mapcache.o -MD -MP -MF $depbase.Tpo -c -o mapcache.o mapcache.c &&\
mv -f $depbase.Tpo $depbase.Po
mapcache.c: In function ‘a_mapcache_init’:
mapcache.c:67:3: warning: ‘g_mutex_new’ is deprecated (declared at
/usr/include/glib-2.0/glib/deprecated/gthread.h:272)
[-Wdeprecated-declarations]
mc_mutex = g_mutex_new();
^
mapcache.c: In function ‘cache_add’:
mapcache.c:73:8: error: void value not ignored as it ought to be
if ( g_hash_table_insert ( cache, key, pixbuf ) ) {
^
make[3]: *** [mapcache.o] Error 1
$ /lib/libc.so.6
GNU C Library (GNU libc) stable release version 2.18 (git ), by Roland
McGrath et al.
Indeed, at least some glib versions,
https://developer.gnome.org/glib/2.26/glib-Hash-Tables.html#g-hash-table-insert,
return void for g_hash_table_insert, so did a cache_remove as first step
op the cache_add.
While doing so I saw this g_hash_table_insert problem,
https://bugzilla.gnome.org/show_bug.cgi?id=692815, could it be related
to the root cause of problems?
> However since the map cache is 'working' well enough, I'm
concentrating on adding other features.
Sure, let me see how stable it is for me and if not, try to come up with
a patch.
Mike.
---
>> From: rw_nor...@hotmail.com
>> To: viking-devel@lists.sourceforge.net
>> Date: Sun, 13 Jul 2014 12:16:51 +0100
>> Subject: Re: [Viking-devel] Segmentation fault with git HEAD
>>
>>> Date: Sun, 22 Jun 2014 13:35:49 +0200
>>> From: em...@gmx.net
>>> To: viking-devel@lists.sourceforge.net
>>> Subject: [Viking-devel] Segmentation fault with git HEAD
>>>
>>> Not sure if the intention is that the Viking git master branch is
>>> always clean but I did try it anyhow.
>>>
>>> I compiled the code based on git index 4200b261[*] and see that I can
>>> easily trigger a Segmentation fault:
>>>
>>> Program received signal SIGSEGV, Segmentation fault.
>>> [Switching to Thread 0x9f851b40 (LWP 6998)]
>>> 0x080b41b6 in a_mapcache_remove_all_shrinkfactors (x=x@entry=1051,
>>> y=y@entry=675, z=0, type=13, zoom=zoom@entry=6) at mapcache.c:187
>>> 187 if ( strncmp(tmp->key, key, len) == 0 )
>>>
>> We've traced this due to the variable tmp being NULL, thus trying to perform
>> tmp->key results in the crash.
>>
>> Further I don't understand the circumstances that leads to this condition,
>> however it has caused me to look at the code in mapcache in general.
> I have committed a basic fix for this which checks tmp before trying to use
> it.
>
>> There are two data structures in the mapcache:
>> 1. An actual hash containing keys against map tiles - this makes sense.
>> 2. A circular linked list queue (using own implementation - I don't think
>> there is a glib support for ringbuffers)
>>
>> I don't understand the point of the second one. The queue seems to be
>> equivalent of the hash cache and used just to compare the keys stored in
>> deciding which ones can be removed.
>> As far as I can tell the keys in the queue are not ordered in any way, so
>> there is *no* efficiency gain compared to just comparing every entry in the
>> hash cache directly.
>>
> After further thought, I now believe the second data structure has use of a
> LIFO queue, so that when the maximum cache size is reached it removes the
> oldest tiles (in terms of being added to the map cache).
>
> For the complication of the code this LIFO method creates, one might instead
> simply flush the entire memory cache and allow it to build up again. The user
> is unlikely to notice any difference.
>
> However since the map cache is 'working' well enough, I'm concentrating on
> adding other features.
>
>
>
>
> ------------------------------------------------------------------------------
> Want fast and easy access to all the code in your enterprise? Index and
> search up to 200,000 lines of code with a free copy of Black Duck
> Code Sight - the same software that powers the world's largest code
> search on Ohloh, the Black Duck Open Hub! Try it now.
> http://p.sf.net/sfu/bds
> _______________________________________________
> Viking-devel mailing list
> Viking-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/viking-devel
> Viking home page: http://viking.sf.net/
>
------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
