Option to prevent a node from being cached when calling node_load()

HEAD is still D7, and your performance patch is still eligible until November 15, IIRC.

I doubt this will go in for D6, but you might take a look at the DBTNG docs here: http://drupal.org/node/310069 --It's pretty easy if you've ever used other frameworks with query builders (CakePHP, Kohana, CodeIgniter, etc.)

@brianV: If you have a patch you want reviewed for consideration into core, you need to set the status of the issue to "needs review"...

Anyway, looking at the patch, I'm not psyched about the two slightly different solutions to the same problem: $reset = TRUE and $cache = FALSE. That seems like a big DX WTF. The $reset flag is exactly for the case you describe: an update that's loading tons of nodes. I'm not convinced that the "I don't like the idea of dumping nodes that may have been legitimately cached by other processes, and which make good sense to cache" concern outweighs the downside of adding a whole new argument for developers to consider when using this API.

Also, definitely no chance of changing the node_load() API in D6. If you're interested in getting this into core, your only chance at this point is D7/HEAD as a performance fix, but I doubt you're going to get much support for this approach. Really, the whole node_load() API (now part of the larger entity_load() API) is pretty set, and the $reset flag is the suggested solution to this problem, and I don't see any major benefit in adding this parallel solution...

Moving to D7.

I agree with Brian, for what it's worth (I'm used to having just another unpopular opinion in the eyes of many [much more descriptive and colorful metaphor elided to avoid offending some and confusing non-native English speakers]).

I might even go a step further -- $cache is already a DX WTF*: node_load() should be about loading nodes, not dumping cache. It's probably just one of many historic DX WTFs all over Drupal core.

Great coinage, Derek, if that's yours.

*(for our international audience) a "cause of developer confusion and anguish" abbreviated into an American english colloquialism.

my general understanding of things is that:

node_load can't really work for a large number of nodes--basically I think this ends up doing a "SELECT *" for every node in your loop, meaning you are A) transmitting a TON of superfluous data and B) you are doing N database queries (opposed to selecting only the needed columns in one query). so my thought is that node_load slowness doesn't really have to do w/ caching.

with that in mind, you have I think a couple of options,
1) use the views module--which is basically generating SQL for you
2) write your own SQL query--especially if you need to "UPDATE"

I'm not that up on OOP or PHP 5 objects, so there might be other very fast ways to do things...with iterators perhaps--but in general, writing my own SQL queries has been the best thing for my performance issues.

side note, I thought cache_clear() dumped the cache whereas RESET meant don't access the cache and then cache the new value (ie, where does dump the whole cache come in?)

but maybe I've misunderstood the use case here, could you post a code/psuedo-code sample?

Just seen this - I'd much rather we did #375494: Restrict the number of nodes held in the node_load() static cache in the entity loader, which would solve this problem generically.

@brianv

thanks for clearing up my confusion on the RESET parameter.

I'll try to restate my suggestion, it seems to me that you are trying to optimize a costly sequential operation and in doing so are creating collateral damage in the cache. As an alternative, you might want to increase the "throughput" so to speak. Meaning, would it be possible to use one massive "update" SQL statement instead of 10,000 individual "select"s + 10,000 individual "update"s (or "delete" and "insert" combos)?

I realize you are not asking for alternative solutions, I'm just trying to think of a way you could improve performance without changing the API.

Why not move the code that loads the node out of node_load() to, say, node_load_nc() ("no cache" -- couldn't think of a better name for now), and call the latter from the former when needed?

E.g.:

function node_load_nc($param = array(), $revision = NULL)
{
    // Handle all module loading stuff, call nodeapi hooks etc.
    return $node;
}

function node_load(($param = array(), $revision = NULL, $reset = NULL)
{
     static $nodes = array();

     if ($reset) {
        $nodes = array();
     }

     // check if node is cached -- return cached version it it's there
     (...)
     if ( $cached_node_found )
          return $cached_node_found;

     // Oops... not in cache -- let's load the node
     $node = node_load_nc($param, $revision);

     // Now add the node to the cache
     if ($cachable) {
      $nodes[$node->nid] = is_object($node) ? drupal_clone($node) : $node;
    }

    return $node;
}

This way, backward compatibility is completely preserved, and modules wanting to just load (perhaps several) nodes would then call node_load_nc().