Problem/Motivation

Book module's template function to load it's prev, next, up navigational structure is a pretty big performance hit (and rarely changes). When it does change, we know what the result of the book_prev, book_parent, and book_next functions will be. On any page once it's calculated once, we shouldn't need to do it again. Right now these are calculated every time, every page in a book, without the ability to cache this blob of data.

This was discovered as a result of a blog post I did on profiling drupal's footprint on page load -- https://drupal.psu.edu/blog/post/memory-profiling-hooks in which book_node_view was found to have a pretty large memory footprint.

Proposed resolution

Implement functionality currently residing on the book_cache module.

Remaining tasks

  • Implement a cache_book cache bin similar to what is being accomplished via the book_cache module
  • Test that this doesn't negatively impact rendering and all use-cases are being accounted for when it comes to times to expire this (or parts of this) cache bin.

User interface changes

None

API changes

None

Data model changes

Adds a cache bin for book module so that it can cache and expire this data correctly.

Patch forth coming.

CommentFileSizeAuthor
#3 drupal-core-optimize-book-module-2538782-2.patch7.03 KBbtopro
#1 drupal-core-optimize-book-module-2538782-1.patch6.9 KBbtopro

Comments

btopro’s picture

btopro commented
Status: Active » Needs review
StatusFileSize
new drupal-core-optimize-book-module-2538782-1.patch6.9 KB

proposed patch by backporting functionality from book_cache module

Status: Needs review » Needs work

The last submitted patch, 1: drupal-core-optimize-book-module-2538782-1.patch, failed testing.

btopro’s picture

btopro commented
Status: Needs work » Needs review
StatusFileSize
new drupal-core-optimize-book-module-2538782-2.patch7.03 KB

Resolved notice that popped up from parent, cleaned up comment style slightly

cilefen’s picture

cilefen commented

Is this an issue in D8 too?

btopro’s picture

btopro commented

template_preprocess_book_navigation appears to be written the same way in D8... not sure the gains would be the same though (or nessecary) given that render_cache will cache what the output was going to be for this operation. It's effectively a render_cache of the details leading up to building this page. Would need some deeper profiling to see. In D7 the menu operations for this one function were like 500kb. This might still be the case in D8 but if its render cached (and always will be after initial build) I don't know what the point of tuning this function would be.

cilefen’s picture

cilefen commented

I was wondering if something like that is the case—already handled in D8 by the advanced caching.

btopro’s picture

btopro commented

would need someone else to step in and speak to that but that's my thinking at least. This is caching the associations of the menu table that are unique to books (who's next, previous, parent directly above me, tree below). While this could still be nice to have ahead of time, render cache would most likely expire in the event the node / surrounding menu items have changed... which would expire the cache_book... which would rebuild the cache_book and rebuild render_cache.

So at least in my understanding of it this wouldn't provide gains in D8 or excessively minimal at best.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.