Problem/Motivation


  /**
   * {@inheritdoc}
   */
  public function render($hook, array $variables) {
    return $this->theme($hook, $variables);
  }

This adds one additional level of function nesting for absolute no win

Proposed resolution

Move the code from ::theme() to ::render()

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 2531972-2.patch1.05 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Title: Remove ThemeManager::theme() and just use ThemeManager::render() » Move ThemeManager::theme() to ThemeManager::render()

Better title

dawehner’s picture

Let's just do that.

Its not nothing

=== 8.0.x..2531972 compared (55ae17109e6d2..55ae174791d07):

ct  : 27,935|27,910|-25|-0.1%
Fabianx’s picture

Status: Active » Reviewed & tested by the community

RTBC, that is indeed unnecessary indirection as small as it can be, it can add up.

dawehner’s picture

Well, one thing what this certainly improves is a little bit less noise in the profiling itself.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Well, one thing what this certainly improves is a little bit less noise in the profiling itself.

Yes I really appreciate changes like this, makes it a lot easier to spot real problems rather than constantly trying to take red herrings into account.

Committed/pushed to 8.0.x, thanks!

  • catch committed 4b36aab on 8.0.x
    Issue #2531972 by dawehner: Move ThemeManager::theme() to ThemeManager::...
Wim Leers’s picture

Issue tags: +Performance, +Profilability

OMG YAY!

@dawehner++

star-szr’s picture

Sorry, posting from my phone otherwise I'd grep but this might need a follow up to update docs pointing to the theme method.

star-szr’s picture

Nevermind, carry on :)

Status: Fixed » Closed (fixed)

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