Optimize rdf.module for displaying multiple comments

This patch introduces $variables['comment_url'] (in the same fashion as node_url) and saves a call to l() in template_preprocess_comment(). The comment_url value is then used in rdf_preprocess_comment(). In total we're saving 100 calls to url().

- rdf_preprocess_comment() still contains a call to url() for the relation to the node. I was not able to use the same technique as for the comment link because node_url is only present in hook_preprocess_node(), and not in hook_preprocess_comment(). There is also another call to url() for the link to the parent comment.
- rdf_preprocess_username() contains one call to url for the user profile page.
- rdf_rdfa_attributes() has to convert the date in iso8601 and relies on the PHP date() function, so there isn't much we can do here, unless we cache it in the db but I'm not sure if it'd be faster.

Random thoughts. Maybe rdf.module should implement its own uri caching. Could we build the comment_url without using url()? you can't possibly put a comment/2#comment-2 in path.module so we don't gain much by using url(). Am I missing another benefit url() brings us here? (beside the base_path which we can get for much cheaper anyways).

quick and dirty implementation of a cache for url(), caching only those urls without query parameter. A quick benchmark shows ~1ms/117ms speed improvement on HEAD with rdf.module disabled, and 3.5ms/136ms (2.5%) with rdf.module enabled.

I measure 74 cache hits with rdf.module off, and 242 with rdf module on.

only non absolute urls with no query parameter are statically cached (keeping the cache overhead as small as possible).

I ran some more detailed benchmarks with 50 and 300 comments.

       |  50 w/o RDF | 300 w/0 RDF |  50 w/ RDF | 300 w/ RDF |
|HEAD  |     121.7ms |     592.7ms |    139.6ms |    714.8ms |
|patch |     121.7ms |     580.4ms |    137.2ms |    696.5ms |
|hits  |          52 |         302 |        214 |       1249 |
| %    |          0% |         -2% |      -1.7% |      -2.5% |
(negative % is faster)

The performance gain is 1.7% with rdf enabled on a 50 comments node page. when displaying 300 comments, more than 2% improvement can be measured for both rdf on and off.

What's killing us next is drupal_attributes() and its call to check_plain() which is called by theme_rdf_metadata() and theme_rdf_template_variable_wrapper().
This sanitize on output rule really slows us down here. There is no user input, and we could require for contrib to sanitize their attributes to save all these calls to drupal_attributes/check_plain.

trying a new approach: caching the values needed to render the RDFa of each comments, that is the results of url() and PHP date(). all cached comment attribute values are retrieved using cache_get_multiple(). (using the 'cache' bin for now for testing purposes). I've measured a 3.1% performance improvement on displaying a node with 50 comments, and 6% on a node with 300 comments.

combining #3 and improved #9 allows us to spare one variable in the cache.

HEAD: 112ms
HEAD w/ RDF: 130.5
HEAD w/ RDF w/ patch: 123.4

this patch brings 5% improvement on a node with 50 comments.

@catch: do you favor a cache_set() per node instead? I thought of it but found it clunky to store comment data on a per node cache record. Or do you suggest to drop the persistent caching idea altogether? I'm not 100% happy with it either but it's the only way I found to optimize further rdf.module. We're relying on API functions (drupal_attributes, check_plain, theme layer) which are inherently slow, so I'm not sure where to look next :(

that's a lot of inserts

Forcing caches misses, i.e. 1 cache_get() and 50 cached_set(), gives a page load of 153ms to be compared with 124ms on warm caches with this patch. 153ms is only when you fill the caches on the first page load. Once all these cache inserts have been done, it's only one big select with cache_get_multiple(). Note that the data being cached is not user specific (no timezone in the ISO 8601 date for example) and can be reused for anonymous and logged in users, not matter what their settings are. Each comment cache is [BLOB - 260B], to be multiplied by 50 when loading 50 comments.

From catch in IRC:
[09:42] scor: OK that doesn't look too bad - I want to see what moshe thinks of this since he's been speaking out against adding more persistent caching recently.

thanks for the useful directions moshe and catch. will work on adding these 3 variables in $comment. in the meantime, could I get a review of the patch #3? I will keep using this approach which leads to less calls to url(), comment_url will be provided in $comment and used directly in template_preprocess_comment().

related thought: we should have a way to pass the output of url() to l() and skip the call to url() in l(). Afaik it's not possible today, that's why we can't use l() in #3 and we have to hard code the HTML code for an A tag...

simpler approach with no persistent caching, but instead we include the date attributes, parent node uri and parent comment uri in the $comment object ($comment->rdf_data), allowing the entity cache module to cache these values.

@moshe: yes, I'm working on a patch for a separate issue, since it is broader than rdf per se (we should probably do it on a per entity basis). see also related full url() support for comment path callbacks cachable with entitycache at #525622-34: 'as link' formatters need a generic way to build the url of an 'entity'

see #699440: Add bundle support to entity_uri callback to remove performance overhead of forum_url_outbound_alter() (regression from D6) for a more generic approach.

Now that some of the aspects of this issue have been generalized and integrated in #699440: Add bundle support to entity_uri callback to remove performance overhead of forum_url_outbound_alter() (regression from D6), it makes this patch even more simple. It does not bring any performance impact in core, but makes it possible for entitycache to cache these values.

Note that this patch does not depend on #69940: Streamlined eCommerce purchase for single item (site subscription) or any other patch and #29 can be committed any time. I've put together a quick'n dirty persistent cache in comment_load_multiple() for proof of concept only, with benchmarks below:
HEAD 136.5ms
patch 132.7ms
2.8% faster

Similar performance gain should be measured when the entitycache module is functional.

rerolling #29