[postgis-devel] Question on "libpgcommon/lwgeom_transform.c"

[Stephen Woodbridge]

Hi Guys,

On my continuing review of "libpgcommon/lwgeom_transform.c" in an effort 
to create something similar for the address standardizer, you are 
getting a detailed code review.

The current question is related to the usage of 
PROJ4Cache->PROJ4SRSCacheCount in:

AddToPROJ4SRSCache(PROJ4PortalCache *PROJ4Cache, int srid, int other_srid)

It looks like the usage of this has morphed over time as it does not 
seem to reflect what it's name intends.

At line 511, we use it to check if the cache is full
At line 523, we use it to remember which item we just deleted
At lines 553-555, we use it to fill the slot just deleted
At line 556, we have a problem!

If we delete any entry less than PROJ4_CACHE_ITEMS - 2, then 
PROJ4Cache->PROJ4SRSCacheCount++ will be less than a full cache and the 
next time we will fill that slot regardless, even if it is other_srid.

While this is not a fatal error, I do not think it is what was intended 
and it leads to some confusion when trying to understand the code.

I'll bug this if you concur with this analysis.

I am thinking that I would like to implement a LRU algorithm for my 
replacement algorithm.

I'm thinking of changing the counter to a index pointer to the next 
available slot. I think I can do this with:

index = (index + 1) % NUM_CACHE_ITEMS;

This isn't an LRU, but rather Least Recently Added (LRA) algorithm.

To implement a LRU algorithm I would need some like

int lru[NUM_CACHE_ITEMS];

then when a slot is used set the lru[i] = 0; and increment all other 
slots (watching for integer overflow). Then when you need to add a new 
slot, you search for the highest lru[i] and use that slot and update the 
slot ranks.

I like the both these, LRA is simple to implement but the LRU might give 
better results in a web portal like application. I think wrapping this 
in a function would allow the algorithm to be changed without impacting 
the code.

Thoughts,
   -Steve