First of all, GREAT MODULE! I'd love to see this api continue to expand, and am hoping the following might make it in.

I have the unfortunate need to run a non-trivial amount of repetitive queries to permissions_get_role() and permissions_get_permissions_for_role(); and am unable to store the results from a single call to these functions locally due to the nature of how i have to process the fields i need to check permissions, so in order to reduce the db load, I decided to implement static caching for these two functions.

Patch provided in first comment.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jwilson3’s picture

ebeyrent’s picture

Thank you for your kind words, and for the patch.

Is query caching on the database not an option? It seems that if you're performing the exact same query, you should enable MySQL's query_cache.

I don't like using static variables in functions without providing a manner of resetting the variable or controlling whether or not the variable should be cached. To do this, we could add a new parameter to the function, $reset=true, and the function would make its database call unless that parameter was set to false.

I am reluctant to make this change though, and I am curious as to why this can't be done on the application side. Design-wise, it seems to me that the API shouldn't really care how it's being used. I mean, these aren't big or intensive queries we're talking about. It seems that the optimization should occur in your application, not in this API.

jwilson3’s picture

Database-level caching I've heard is not so good to depend on; going down to the db layer ~60 times on every page rendering, even if query caching is on, is still less efficient than caching at the API layer. I had already ruled out caching at the application level in my OP although now that i think about it, it is probably possible. I just figured a performance tweak like this would be an easy win to install at the API level for the benefit of the masses. But based on your reluctance, would it even be worthwhile to create a new patch that had cache expire support?

FWIW, I totally agree that there should be a way to reset the static cached variable. I thought about that only after creating the patch. :/

ebeyrent’s picture

Status: Needs review » Closed (won't fix)