Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.http://api.drupal.org/api/function/template_preprocess_node/7
and
http://api.drupal.org/api/function/theme_node_submitted
both do format_date($node->submitted);
Those 20 calls to format_date() on a default front page account for 5.5% of execution time.
We should either split $submitted into author and date, or remove $date and let themes handle it themselves - but doing this work twice when we only use one variable in any one template is a waste.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | format_date.patch | 1.27 KB | catch |
| #6 | head.png | 31.49 KB | catch |
| #6 | static.png | 25.19 KB | catch |
| #2 | comments_too.png | 42.21 KB | catch |
| #1 | format_date.patch | 682 bytes | catch |
Comments
Comment #1
catchLet's try just removing it first. There must be an existing issue somewhere for making $submitted less horrible.
10 nodes on front page. ab -c1 -n500
HEAD:
8.92 [#/sec]
8.80 [#/sec]
9.20 [#/sec]
Patch:
9.37[#/sec]
9.40 [#/sec]
9.46 [#/sec]
Benchmarks come out more like 6% compared to the expected 2.5% from profiling, but close enough and it's definitely a measurable cost.
Comment #2
catchComments have the same problem.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedNice work focusing on this function. Why is it such a pig? I would have thought that the new Date API in PHP is fast.
Somehow, we need to get to a pull system where we only prepare what the theme needs.
Comment #4
Dries CreditAttribution: Dries commentedWow, that is a surprise. This sounds like RTBC to me, but I agree that a pull system would probably by desired at some point.
Comment #5
catch#243129: Refactor format_date() to avoid multiple calls to date_format() has been around or a while but format_date() has changed significantly since then so it may not be the same issue any more.
I thought about adding a static cache to format_date() as well - would like to try that before we remove the variable.
Comment #6
catchHere's a try with a static. Didn't seem to be any need to use drupal_static() here - can't see the static needing to be cleared but I might've overlooked something.
Attached webgrind screenshots.
Comment #7
Damien Tournoud CreditAttribution: Damien Tournoud commented#243129: Refactor format_date() to avoid multiple calls to date_format() is still needed. Instead of adding a static in format_date(), couldn't we avoid the duplicate call inside the field api?
Comment #8
catchIt's not field API though - it's template_preprocess_node/comment creating their own date variables as well as using the _submitted theme functions.
Removing $date from template_preprocess_node() (and the same with comments) removes the duplication in HEAD - but I can see themes adding that back in again all over the place,. Since Drupal 6 has the same issue, we could potentially backport a static (300 extra calls to format_date() on a long thread is potentially enough to warrant it).
Ideal thing for HEAD would be for format_date() to not be such a beast, but no-one's working on that issue and I'm not really up on date handling enough to tackle it, so it's a case of whether we do something interim or try to focus efforts over there.
Comment #9
JohnAlbinYeah, themes are always wanting to do different date formats because every site owner wants their own bikeshed.
Since that new static cache patch landed, this patch needs to be re-worked to use that method, no? /me admits to being too sleepy to look it up at the 'mo. Leaving at "needs review" for now.
Comment #10
catchPostponing this on these two:
#243129: Refactor format_date() to avoid multiple calls to date_format()
#216961: Make author/submitted by separate toggles
Comment #11
effulgentsia CreditAttribution: effulgentsia commentedUn-postponing. @catch: what's your current thinking on this?
Comment #12
retester2010 CreditAttribution: retester2010 commented#6: format_date.patch queued for re-testing.