Cache the sets of JavaScript/CSS in drupal_get_library()
Rob Loach - July 4, 2009 - 21:11
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | javascript |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | won't fix |
| Issue tags: | drupal_add_library, Libraries |
Description
#315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries) allowed modules to register sets of JavaScript/CSS libraries. The problem is that it's not cached. This means that if a contributed module did something really expensive with the registry, like searching for libraries on the harddrive, or did a complex alterations of them, it could potentially be a big performance hit. This issue is to cache that registry so that we don't run into these performance implications.
| Attachment | Size |
|---|---|
| drupal.get_.library.cache_.patch | 5.58 KB |
| Testbed results | ||
|---|---|---|
| drupal.get_.library.cache_.patch | failed | Failed: 12458 passes, 11 fails, 0 exceptions Detailed results |
#1
On one hand, you're right that there might be a module that performs heavy actions to alter libraries. Libraries API might be a good example. On the other hand, a module like that most probably has to perform many other additional actions anyway, so it needs it separate caching (and controlled cache flushing) anyway.
+ if (isset($module)) {+ return isset($libraries[$module][$name]) ? $libraries[$module][$name] : FALSE;
}
else {
- $libraries[$module][$name] = FALSE;
+ return $libraries;
}
Usage of $name leads to a PHP notice here.
#2
Tagging.
#3
#505084: Add #attached_library FAPI property for drupal_add_library()
#4
Updates to HEAD, takes sun's note about the $name into account and fixes a namespace issue.
#5
/me subscribes
#6
The last submitted patch failed testing.
#7
HEAD is broken.
#8
Really expensive libraries can use own caching. We have to end the cache explosion soon. -1.
#9
Moshe, what about modules that regex a file to find out what version of the library they have? To improve the developer experience (modules wouldn't have to cache the data themselves), as well as improve performance (calling a number of module hooks which could do expensive things like regex an opened file), we could just easily cache the library array.
#10
Neither of those arguments is compelling. Adding caches rarely improves developer experience. You then have to understand what cache to clear when you are developing and debugging ... Further, the performance gain here is insignificant. It *only* could happen for an expensive operation when its module author cannot be bothered to do own caching.
This is a speculative cache. It might help under certain circumstances that are theoretical. Drupal has enough caches without implementing speculative ones.