Side-effect caching support

David Strauss - February 5, 2007 - 02:04
Project:Drupal
Version:7.x-dev
Component:base system
Category:feature request
Priority:normal
Assigned:David Strauss
Status:needs work
Description

Currently, caching has limited easy use because side-effecting operations are not repeated when the cached item is loaded.

For example, a function building the front page might return the content but also add a stylesheet and an RSS feed header. If only the results of the front-page-building function are cached, loading the cached version requires either manually repeating the side effects or awkwardly storing the side-effects in the cache for replay. As function calls get deeper, predicting and replaying these side effects becomes very difficult.

We could improve such caching by having a hook_side_effect that is called whenever side-effecting operations run. hook_side_effect would be called with the function name (e.g. 'drupal_add_js') and the array of arguments the function received. Caching modules could implement the hook to record and replay side effects when the cached item is retrieved.

#1

David Strauss - February 6, 2007 - 06:48
Title:Side-effect function hooks for caching» The addition

Add this:

module_invoke_all('side_effect', __FUNCTION__, func_get_args());

to following are side-effecting functions:

  • drupal_add_css
  • drupal_add_feed
  • drupal_add_js
  • drupal_add_link

This will allow caching modules to log and replay side-effects on cache restoration.

#2

David Strauss - February 6, 2007 - 06:51
Title:The addition» Side-effect caching support

Whoops, didn't notice that the title changed the entire bug/feature report. Should have read the descriptive text...

#3

David Strauss - February 6, 2007 - 07:05
Assigned to:Anonymous» David Strauss
Status:active» needs review

Here's a patch to a current HEAD checkout. I tried to minimize the hook calls to when actual effects happen. I'm sure this could be improved further.

AttachmentSize
side_effect_cache.patch 1.19 KB

#4

David Strauss - February 6, 2007 - 07:08

I'd like feedback over whether its better to implement this as a hook or to simply compare the arrays before a function call and after to identify additions. The latter would be more complicated but possibly faster in the common case (no cache).

#5

David Strauss - February 6, 2007 - 07:29

Given the arbitrary values for $scope for drupal_get_js() and the inability to simply load the whole array for all scopes, it seems impossible to determine what core side-effects a function has used by probing for changes.

#6

David Strauss - February 6, 2007 - 07:48

Here's an alternative patch that at least allows retrieving the entire array of JavaScript for all scopes. This has a clear performance advantage over adding hook_side_effect, but it's not as extensible. hook_side_effect could be used to record arbitrary side-effecting calls. This alternative patch would require the cache to probe for side effects in specific places.

Of course, a hybrid approach is possible, too. The cache could probe core modules for side effects but still implement hook_side_effect for arbitrary additional effects.

AttachmentSize
alt_side_effect_detection.patch 1.03 KB

#7

RobRoy - February 6, 2007 - 19:36
Status:needs review» needs work

You got a TAB in there. Use two spaces instead for indents.

#8

David Strauss - February 6, 2007 - 20:01

Updated to remove tab.

AttachmentSize
js_side_effect_detection.patch 1.03 KB

#9

David Strauss - February 6, 2007 - 20:02
Status:needs work» needs review

#10

RobRoy - February 6, 2007 - 20:05
Status:needs review» needs work

Heh, you added some trailing whitespace in there now. Not trying to be a pain in the ass, I swear.

#11

David Strauss - February 6, 2007 - 20:18
Status:needs work» needs review

No biggie. Whitespace removed.

AttachmentSize
js_side_effect_detection_0.patch 1.03 KB

#12

catch - October 24, 2007 - 11:18
Status:needs review» needs work

No longer applies.

#13

Pancho - February 3, 2008 - 16:18
Version:6.x-dev» 7.x-dev

Not sure about this. I like the more extensible original idea. This would be an API change though. Tempted to move this to the D7 queue.

 
 

Drupal is a registered trademark of Dries Buytaert.