We're working to make all blocks cacheable. This block can trivially be made cacheable. Let's just get it done.

CommentFileSizeAuthor
#6 cacheable_syndicate_block-2232385-6.patch1.13 KBwim leers
#1 cacheable_syndicate_block-2232385-1.patch866 byteswim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new cacheable_syndicate_block-2232385-1.patch866 bytes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Spark, +sprint
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Quick fix

We're working to make all blocks cacheable. This block can trivially be made cacheable. Let's just get it done.

Agreed so RTBC.

znerol’s picture

znerol commented

Perhaps this is a stupid question, but what is the performance benefit of caching this block? If I'm not mistaken this will generate an additional trip to the cache backend (which possibly might be the database), while rendering it is just a matter of feeding some static array into drupal_render. Has the render API really become slower than the database? Do we have benchmarks for that?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs review

Good question.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Cacheable "Syndicate" block » Cacheable "Syndicate" block: never cache it, because it's cheaper to render
StatusFileSize
new cacheable_syndicate_block-2232385-6.patch1.13 KB

I was in a "convert all the blocks" frenzy, but I agree. In fact, we should do precisely the opposite: prevent caching, because it's so ridiculously cheap to render. A cache get implies I/O, and I/O is always going to be more expensive than a very limited number of CPU cycles. Therefore, avoid I/O altogether.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Cacheable "Syndicate" block: never cache it, because it's cheaper to render » Cacheable "Syndicate" block: never cache it, because it's cheaper to always render it
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Really wise observation!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit 0e21df5 on 8.x by webchick: 
    Issue #2232385 by Wim Leers: Cacheable 'Syndicate' block: never cache it...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: -sprint

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.