KML module should cache output.

I completely agree that we need to cache the output, but we need to make sure that access controls (node_access and user permissions) are respected too, so it maybe should be a cache per user? For sites that are used mostly by anonymous users, that should reduce load quite dramatically.

Re: #1 - yes... very good point. Having a cache per user would involve a huge storage overhead though.

I have slapped together a caching patch but it doesn't take account of user access so will not be useful for everyone. It makes a big difference to the kml/node path though so clearly a good idea.

Hmmmm... any suggestions on a way forward?

In response to my previous question about how to cache nodes depending on what users are allowed to access...

The first attempt at a cache patch did not take into account a user's node access. The CID of the cache element was simply something like kml:node or kml:term:59.

This is all well and good for all available nodes, or nodes in that term but does not account for when a user cannot see nodes through access rules (using OG, taxonomy access control, etc).

Out of interest I node that the KML module does not check read access using node_access but does some sort of check for groups.

I considered one possible option that might work, but the node_access checks may defeat the point of a cache (see drawbacks). I'm just writing about it here to stimulate conversation I guess. ;)

So, if one generated a list of visible NIDs for each user's access, sorted and concatenated that list and made hash of them it could be used in the CID.

The hash would be unique to the node list available to any one user, but many users would probably have the same access rights so would share the hash and therefore be able to make use of the cache. The CID then may become, kml:node:18e8d39c806970d663ca3991ddff0fa7 or kml:term:59:18e8d39c806970d663ca3991ddff0fa7.

Two drawbacks:

1. This means always building a list of visible nodes and calling node_access on each.
2. When nodes are added (or removed) the hash would change. Clearly the cache would build up over time with different hashes so a maximum cache lifetime would have to be enforced to prevent this build up. There should be a function to reset the expire date when we use a cache entry that is useful.

I'm not convinced that any of this is a good idea based on drawback number 1, but as I said, just throwing ideas around right now.

Re: #3: Wow! Nope, drawback #1 is not an issue. On examination of the node_access function if one fetches nid, type, status and uid then there is no need to fetch the rest of the node before calling node_access.

I modified the KML module to fetch these elements for target nodes and compare the changed times as it went (for comparison to any cache built to see if it is out of date). This list of nodes is then passed down into the rest of the functions, the rest of the node data being fetched later with node_load of course.

On a test system with over 1000 spatial nodes standard build time for kml/node is 6-7 seconds. Once cached this takes less than half a second to return.

Beginnings on this committed:

http://drupal.org/cvs?commit=131697