Restrict the number of nodes held in the node_load() static cache

I support this for the very reason you mentioned. Running any module with a large number of nodes, fails with blowing PHP's memory limit.

The node module is to blame, but equally the function taxonomy_get_tree() and taxonomy_term_count_nodes() are big culprits. So perhaps you can include them in your patch?

A few minor things...

+    $nodes+= $queried_nodes;

- Missing whitespace...

// TODO: These messages need to be removed before this is committed.
drupal_set_message(format_plural(count($node_cache)-NODE_LOAD_MAX_CACHE, 'Purge @count node from static cache', 'Purge @count nodes from static cache'), 'warning');

- You shouldn't add stuff to patches that shouldn't be commited :)

- Whouldn't array_slce() be easier to use, given that we only need one chunk? http://ch.php.net/manual/en/function.array-slice.php

Had been discussing a similar thing with Damien recently but I don't think there's an issue yet. This could also be centralized for all first class objects if #460320: Standarized, pluggable entity loading (nodes, users, taxonomy, files, comments) gets in.

Indeed, we discussed with catch implementing an in-memory LRU cache. I even did some testing, but failed to open an issue yet.

If one is doing strange things like iterating over all nodes, then one can clear the cache every so often. once might argue that this is too specialized for core. i'm not very opinionated either way.

OK, I remove my objections. This is a good idea.

I would love to change the constant to a variable_get() so that sites can change the cache size as needed. A precedent for that is variable_get('session_write_interval', 180) in _drupal_session_write(). I will RTBC after that change.

If the entity loader gets in first, this should move into there.

This should now go into the entity.inc static caching (which is based on node_load_multiple()) so ought to be more or less cut and paste. Only question then is do we restrict the number held per entity type - you might easily have 300 taxonomy terms on a page, compared to 300 nodes for example.

Anyone up for a reroll? I think 100 items is a nice number. Could be lower I suppose.

entity.inc needs the patch now.

Just ran into a huge memory leak when running devel_generate() on article nodes, due to a node_load() in file_field_update(). Here's a re-roll minus the tests putting this in entity.inc just to see what the test bot says.

Minus the notreallycamelcase.

#21: cache_lru.patch queued for re-testing.

Rerolled with tests and even better docs. I could use some help on the remaining test failure. Seems like it is a real bug in the LRU code, not in the test.

Looked at this for a little while with + += array_merge() and array_merge_recursive(), as far as I can see if there's a numeric index, inserting new items will always insert them in numeric order with any of those rather than prepend/append the entire array. Didn't break it down to verify this yet.

Would be good to get this in soon.

Am very interested in a Drupal 6.16 version of this.

I have a large command line script for generating an offline version of a Drupal site with thousands of nodes.

Despite all other memory reduction strategies, the memory usage just grows and grows until the limit is hit,
and as far as I can tell node_load is the culprit.

Perhaps a little of topic, but I am using the following and nevertheless memory grows with every node_load:

   $revision = null;
   $reset_cache = true;
   $node = node_load($nid,$revision,$reset_cache);

Grateful for any feedback.

Moved my questions to support request: #813590: Require assistance with memory usage in node_load

an LRU sounds cool.

Previous patch's test failed because the popular node was only cached once, so it never got a chance to get into the front when loaded repeatedly. We need to cacheSet entities even if they're already in the cache for LRU to work right? Headsmacking oversight I think...

#31: lru-2.patch queued for re-testing.

Re-rolled for the 8.x branch.

Looks proper to me. Nice tests.

-  protected $entityCache;
+  public $entityCache;

Why that?

--- a/includes/common.inc
+++ b/includes/common.inc
@@ -7287,6 +7287,7 @@ function entity_get_info($entity_type = NULL) {
           'fieldable' => FALSE,
           'controller class' => 'DrupalDefaultEntityController',
           'static cache' => TRUE,
+          'static cache max size' => 100,

If we add a new entity info key, we also need to add docs for it.

   /**
-   * Stores entities in the static entity cache.
+   * Stores entities in the static entity cache. Least recently used (LRU) are
+   * purged in order to honor 'static cache max size' property. Use
+   * hook_entity_info_alter() to change this property.

As to the coding standards, the first line of the comment block should be a full sentence.

+      // Add entities to the cache if we are not loading a revision. We need to
+      // cacheSet entities in the cache as well for LRU.

Out of context, this comment makes me just wonder what LRU means.

Subscribe.

Just opened #1199866: Add an in-memory LRU cache, that is not quite ready for here yet, would be good to have the facility to add multiple items to cache at once, just cross-linking for now.

Tagging.

Is this still unbouded? We did add a persistent entity cache recently.

Marking postponed on #1199866: Add an in-memory LRU cache. The latest patch here actually solves this issue too.

Seems the postponed issue landed a while ago.

Not only that, but #3498154: Use LRU Cache for static entity cache landed this year, which is a full on duplicate of this.