The new "all books" version of the book block can have its performance improved because it calls menu_tree_page_data() for every book even though we can determine that at most one of the books will correspond to the current page. For all the other books, we already have all the data we need from book_get_books(0 to build the single-link menu that will come back from menu_tree_page_data() (after 2 uneeded queries).

Attached patch gives the idea, but untested.

CommentFileSizeAuthor
#3 book_block_perform_3.patch3.13 KBpwolanin
#1 book_block_perform_2.patch3.06 KBpwolanin
book_block_perform_1.patch2.89 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
StatusFileSize
new book_block_perform_2.patch3.06 KB

improved - fixed parse error and notice.

pwolanin’s picture

pwolanin commented
Assigned: Unassigned » pwolanin
Status: Needs work » Needs review

Initial testing - this seems to work fine.

pwolanin’s picture

pwolanin commented
StatusFileSize
new book_block_perform_3.patch3.13 KB

per suggestion by webernet and standard Drupal coding standards - moved a code comment to its own line. No other changes.

webernet’s picture

webernet commented

Tested OK - doesn't seem to break anything.

flk’s picture

flk commented
Status: Needs review » Reviewed & tested by the community

tested ok, performance improvement in regards to making menu_tree_all_data call for one book only at a time (the one that corresponds to current page)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Needs work

I have not tested the code, but looking at the code, this line would drop an E_NOTICE error, if not on a node page:

$current_bid = empty($node->book['bid']) ? 0 : $node->book['bid'];
pwolanin’s picture

pwolanin commented
Status: Needs work » Reviewed & tested by the community

@Gabor - I think you are not correct about this - the use of empty() avoids any such notice. This code has been tested on non-node pages, of course.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Indeed, I was mistaken. Overlooked that simple check.

Thanks for the patch, committed!

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)