because drupal_static is cool

Files: 
CommentFileSizeAuthor
#2 features_drupal_static-1983022-2.patch3 KBundertext
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch features_drupal_static-1983022-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
features_drupal_static.patch2.97 KBhefox
PASSED: [[SimpleTest]]: [MySQL] 40 pass(es).
[ View ]

Comments

Angry Dan’s picture

Status:Needs review» Needs work

This patch is out of date and needs calls to drupal_static() to be by reference:

<?php
$processed
= drupal_static(__FUNCTION__, array(), $reset);
?>

Should be (note the &):

<?php
$processed
= &drupal_static(__FUNCTION__, array(), $reset);
?>

But definitely +1 for this - features is a pig for memory, so moving to the static caching system is a good step away to being better able to manage memory usage (via resets).

undertext’s picture

StatusFileSize
new3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch features_drupal_static-1983022-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
undertext’s picture

Status:Needs work» Needs review
bojanz’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community

We have this running in production for months now.

hefox’s picture

Status:Reviewed & tested by the community» Fixed

thanks

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Taran2L’s picture

Don't see this code in the features codebase. Has this patch been committed at all?

Issue #2143765: features_modules_enabled() doesn't work when features_load_feature cache doesn't include the feature got me here. Have the same issue with 7.x-2.2

Taran2L’s picture

Status:Closed (fixed)» Reviewed & tested by the community

Seems like it was closed before actual commit was done. Reverting status back to "Reviewed & tested by the community"

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 2: features_drupal_static-1983022-2.patch, failed testing.

joelpittet’s picture

Issue tags:+Perfomance, +Needs reroll

It looks like this may need a re-roll as well.

lsolesen’s picture

Status:Needs work» Fixed
Issue tags:-Perfomance, -Needs reroll

@joelpittet and @Taran2L I am pretty sure that this has been committed. I tried a reroll just now and ended up with an empty diff. So setting back to Fixed. If you disagree, please reopen again.

joelpittet’s picture

@lsolesen I think you are correct, just the committed code is just slightly different than the patch for the resetting approach and no commit hash showed up in the issue queue.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Taran2L’s picture

Hi @all,

Sorry for the confusion: I've had a false impression, that my issue is cache-related.

Actually, it's related to the fact, that Features module fails to include files when module(s) are being enabled.

See more #2143765-6: features_modules_enabled() doesn't work when features_load_feature cache doesn't include the feature