Improve performance by replacing very small and simple templates and theme function with "Markup Utility Functions"

added "way forward"

added a related issue

Updated issue summary.

#4 -

What's an example of a MUF candidate for which there would even be the possibility of a render array inside?

The "standard" example we've been using is a link wrapping an image, but really any MUF that could wrap a more complex structure that might itself want to be a renderable array is a use-case. Consider a preprocess where a nested array is built up and we don't want to flatten it into a string before it hits the template, if MUFs can't handle the nesting when used in render arrays then we're often forced to fallback to slower alternatives.

There's a few examples of this happening already in the Twig conversion issues with '#theme' => 'link' and it would only happen more if we had more l()-like functions.

What I'm getting at is, is this issue necessary at all, or is it possible to draw a hard line between MUF (with no template override supported at all) and templates? If there are use cases though, then I think the approach in the issue summary is great.

From the work we did recently on l() I believe that it would be possible that we could get away with the following and remove templates for MUFs altogether:

- Render arrays support a '#markup callback' attribute which points to a MUF like 'l', 'tag', 'img', 'mark', etc..
- All MUFs called directly can be asked to return either structured data or a string, making them suitable both for preparing variables in preprocess functions and generating markup directly
- Where a MUF represents an HTML element that may contain more complex structures it must support render arrays passed as it's content
- Other than using drupal_render() to handle non-string content passed in, the MUF must not call any function that would invoke the theme system
- MUFs cannot be overridden but their structure, defined as the data that they return in "structure mode" is alterable

It would mean adding a few lines like this to l() (for example):

if(!is_string($text)) {
  $text = drupal_render($text);
}

From the profiling we did on l(), introducing an alter added about 1% to the time spent in l() see http://drupal.org/node/1922454#comment-7354042

For all the other MUF candidates this would be a performance increase as the alter is still faster than any theme implementation profiled.

#7 - yeah, i think that's a good point but an implementation detail (what you're saying is closer to the original proposal in the issue summary). maybe I should have left it out of what I was saying in #6 because I'm still trying to look at "the forest" here and discussing exactly what functions are responsible for making all this happen and how they do it might get us bogged down in the details really fast :/

@jenlamption -What you were saying in #5 is probably a better discussion point this early, that there are two elephants in the room for Twig performance that are related but distinct:

1. There is "atomic markup" that has no reason to invoke the theme system when an alterable MUF that accepts renderable content would be more than enough flexibility for every imaginable use-case. When I say "atomic markup" I mean a chunk of HTML that is *at most* a single tag and it's wrapper - the tag may have contents of arbitrary complexity but the contents *must* be a string or passed off to drupal_render(), when a wrapper exists the type of tag used for the wrapper (div, span, etc..) would be alterable too.

Whether we choose to let the theme system "intelligently" step aside and fallback to a MUF or we require MUFs to be explicitly declared in our structured-data-to-be-rendered would need discussion/prototyping but it doesn't change the "vibe" of what we're trying to achieve with MUFs in general.

2. If we're being objective, Twig templates don't provide any benefit at all when all you're using them for is to generate *default* markup - 100% of their value-add is for themers looking to override default markup. Core would run much faster if every Twig template in the conversion queue was just a foo.html.twig.example template and had a corresponding "default render" function that uses the variables provided by a preprocess to return a string, bypassing Twig completely - depending how this was implemented it would even be possible that D8 pre-overrides becomes faster than a fresh D7 install as there would be *zero* templates in effect by default.

If others agree that this is a fair assessment of the situation I propose that we split point 2 into a separate issue and focus on point 1 regarding MUFs and "atomic markup" here.

For point number 2 in #8 I opened this #1986804: Twig templates in core should just be examples, all default markup provided by core should be theme functions. and immediately postponed it so we don't derail anything, it's just food for thought for later and not what this issue is about.

Here's a clearer title for the issue too :)

Updated issue summary.

I've just opened #1987114: Allow l() to accept a renderable array for $text as per #6.

@jenlampton - Fabianx and I have been talking a bit more about this in IRC and it looks like our first attempt to implement a MUF into the theme system will be basically what you said in #7 - we'd be mapping our MUFs to theme functions in the registry so '#theme' would just "know" what to do.

Added a relevant l() issue.

* MUFs are called either before or after all preprocess functions.
** Currently voting for: after.

I'm going to go with a vote for before, because otherwise calling a MUF directly could have different behaviour to it being invoked by the theme system due to preprocess functions messing with variables.

#15 -

Would a less-sweeping change (instead of a new MUF API), would be to have more of core (like form fields) work like theme_field() for performance reasons?

The more I think about it I'm kind of going back to what I was proposing in #6 and saying that MUFs should have nothing to do with the theme system. They can't be overridden, just their content + attributes altered and their "structure" is fixed (ie. an "a" tag can't be turned into a "foo" tag). A MUF is an acceptable way to render part of a renderable array, and a renderable array can be the "content" of a MUF - I believe this would be enough to ensure that MUFs are interoperable with theme() without being governed by it.

Remember that l() already exists and has for years. I don't think of this as a "new MUF API" but rather taking one proven convention we already have, giving it a name to help people learn and identify with it and then applying it to other places where we collectively agree that it makes sense to.

For years (D6, D7 and D8) we've had staunch resistance against replacing l() with theme_link() whenever it's been suggested *purely* for performance reasons so I'd expect the same arguments to hold true in reverse when we suggest replacing theme_image() with img() (for example). Many modern sites have 50+ or even hundreds of images being rendered on some pages.

also, I think we actually need performance benchmark numbers to indeed confirm that performance *is* a problem (and the specifics of where it is a problem) so that we are making sure we are fixing problems we actually have.

You're absolutely right, if we're going to add "yet another" way of rendering HTML we need a clear way to define what should use the new system and what shouldn't. If we can't define why we want to use it, we probably shouldn't build it in the first place.

There are two problems we currently have that we could use MUFs to help address (as I understand it):

1. Performance of rendering HTML is a very common Drupal problem. Every millisecond we can shave here counts.
2. "Build to use cases" is a Drupal philosophy that theme functions for individual HTML tags violate. As @jenlampton as re-iterated across many different threads now every single proposed use-case for themeable tags can be covered by simpler/faster alter implementations so there is no use-case to allow themeable tags unless we say "themeability" is an end rather than a means.

Using l() as a reference, check out http://drupal.org/node/1922454#comment-7362682 for benchmarks comparing theme_link() and l(). For the sake of discussion, imagine a parallel universe where everything is run through theme_link() and we're discussing inventing a new l() function (which is basically the current situation for theme_image()).

For 200 links l() = 230ms, l() with an active alter = 233ms while theme_link() which does essentially nothing except call l() + invoke the theme system is 243ms. With a template in place this figure blows up further.

243/230 = a 5.7% performance overhead we incur "just in case" we need something that we have *zero* use cases for. theme_image() does less internal processing than l() so you would expect the % overhead of the theme system to be larger for images than links, although I have no data available to back that up :(

If we go back to the simple '#markup callback' suggestion we could potentially achieve close to 100% of this performance improvement inside renderable arrays "early returning" it. Eg. the top of drupal_render() might look something like this:

function drupal_render(&$elements) {
  // Early-return nothing if user does not have access.
  if (empty($elements) || (isset($elements['#access']) && !$elements['#access'])) {
    return '';
  }

  if(isset($elements['#markup callback'])) {
    return (string) $elements['#markup callback']($elements);
  }

As I understand, a template adds about 10% overhead over a theme function (but darned if I can find where I saw those stats now that I'm looking for them :/) so the overhead of a template vs. a MUF would be 15%+

So, what would this look like (high level):

MUF - single tags or tag + wrapper -> alterable only -> 99% of the time speed is all we care about for a single HTML tag, every millisecond counts.
theme function - could have been a MUF except for the fact that we have a decent/common use-case for providing full themeability -> think views fields, or maybe we just have a few related tags (or a loop like views-fields) that don't fall under the "single tag" rule for MUFs
templates - too complex for your "average" themer to be expected to comfortably manage overriding a theme function for, or maybe there's a good chance they'll punch a security hole in the theme layer if we let them and so we want to enforce auto escaping.
'#type' - self contained "elements" that do some "prerendering" and then might wrap a theme function or MUF for use in the Form API or similar

It's pretty late here so I'm doing this in a hurry, but I did some really prelim benchmarking style testing for images with timers:

  $variables = array(
    'uri' => 'http://drupal.org',
    'width' => 300,
    'height' => 300,
    'alt' => 'some alt text',
    'title' => 'an image title',
    'attributes' => array(
      'class' => 'some class',
      'data-foo' => 'foo',
    ),
  );

  timer_start('foo');
  $i = 0;
  while($i < 1000) {
    $image = theme('image', $variables);
    $i++;
  }

  $end = timer_stop('foo');
  print $image;
  print_r($end);

  timer_start('bar');
  $i = 0;
  while($i < 1000) {
    $image = theme_image($variables);
    $i++;
  }

  $end = timer_stop('bar');
  print $image;
  print_r($end);

Leads to:

<img class="some class" data-foo="foo" typeof="foaf:Image" src="http://drupal.org" width="300" height="300" alt="some alt text" title="an image title" />Array
(
    [count] => 1
    [time] => 128.62
)
<img class="some class" data-foo="foo" src="http://drupal.org" width="300" height="300" alt="some alt text" title="an image title" />Array
(
    [count] => 1
    [time] => 87.31
)

Running the tests with 200 images (about the most you'd expect on a modern site for a single page), I get a pretty consistent ~9ms saving by not using the theme system and calling theme_image() directly. Just working through the theme system (not rendering anything) is like 40%+ of the time spent in theme('image').

just noodling around with ideas for #16.

#20 - cool, I'm following that issue. I also opened #2004072: Add a non-themeable replacement for theme_image() and theme_image_style()

#20 - I've read through the readme at https://github.com/c4rl/renderapi

@c4rl, Would it be fair to say that what I was suggesting in #16, in the language of your new render API proposal would be analogous to saying:

When the base abstraction and the primary abstraction are guaranteed to be identical, ie. the only allowable render keys for *content* (ie. '#style' in the image example on github wouldn't make the base/primary abstractions differ) of a given '#type' are the two reserved ones "attributes" and "inner", there is no need to invoke the theme system to use a template so we don't waste CPU resources doing so (as this could be the case for many hundreds of render calls on a single page load). This is because with no extra allowable render keys we're already looking at the base abstraction and therefore the only further information/themeability the '#type' could give us that we can't achieve through an alter is what type of HTML tag to apply our attributes to eg. "a", "img", "script", etc.. and this could be pre-defined in a '#type' registry somewhere anyway.

Anything that traditionally would have been done processing variables in a theme function, in the case of base abstraction == primary abstraction could be handled by an alter alone. For example, theme_image() which does essentially nothing but attempt to resolve the "src" attribute with file_create_url() could be refactored as an alter for '#type' => 'image' that just processes the "src" attribute by reference.

Traditionally, theme_html_tag() has been leveraged in some places to "intelligently self close" tags passed in, but this logic could be moved into the render API refactor for when base abstraction == primary abstraction for a given '#type' AND there is no "inner" value in the renderable array passed in as an argument.

I had a bit more of a think about MUFs in the context of @c4rl's proposed new render API and thought we could take #22 a step further. Instead of having a bunch of little MUFs running around with their own idiosyncrasies and maintenance overhead, we could inline the logic currently in the theme_html_tag() into drupal_render() as default behaviour of every '#type' where the '#type' is literally the tag (so a link would be '#type' => 'a', image '#type' => 'img', etc..) and then have a "type registry" which maps types to theme functions as desired when we want drupal_render() to support more complex patterns. This way the definition of the theme system (from the perspective of drupal_render()) becomes "the thing that maps the Primary abstraction to the Base abstraction when the type registry indicates that they may be different".

Something (very roughly) like this:

// Inside drupal_render().
$type_registry = get_type_registry();
$type = check_plain($elements['#type']);
// Allow modules to alter $elements before we render.
Drupal::service('module_handler')->alter('type_' . $type, $elements);

// Render unregistered types as single tags.
if(!isset($type_registry[$type])) {
  // Alter could unset or set $elements to NULL to remove this tag from the markup.
  if(!isset($elements)) {
    return '';
  }
  $attributes = isset($elements['attributes']) ? new Attribute($elements['attributes']) : '';
  if(!isset($elements['inner'])) {
    return '<' . $type . $attributes . " />";
  }
  else {
    return '<' . $type . $attributes . '>' . drupal_render($elements['inner']) . '</' . $type . '>';
  }
}
else {
  // Obviously I have no idea what the structure of this registry would end up looking like...
  return $type_registry[$type]['function']($elements);
}

• Always know when something is a renderable array because it has '#type' (something Fabianx mentioned in IRC)
• Supports all current and future HTML tags that would plausibly be introduced by browsers between Drupal major releases in a consistent way
• We can remove every theme function that is a single tag from core and get a modest performance boost by doing so
• In some cases we can combine multiple theme functions (like theme_image_style() and theme_image()) by simply setting a '#style' option and letting the alters respond to it instead of doing what we currently do with multiple passes through the theme system. We obviously expect a proportionally larger performance gain for our efforts in these cases.
• If for some reason we want to add a theme function/template back in for a single tag we just add it to the type registry and the default single tag logic is bypassed completely

tl:dr; - We probably don't need MUFs at all if c4rl's proposed render API is adopted with theme_html_tag() inlined into drupal_render() as default behaviour for unregistered '#type's.

I'm going to close this. The current way forward for issues of single-tag theme functions is to create a #type -> #pre_render -> #markup equivalent render array and delete the theme hook, see #1825090: Remove theme_html_tag() from the theme system, #1985470: Remove theme_link(). It's a crude approach in the scheme of things, but it seems to be getting the job done well enough for now and the approach can be improved upon iteratively over time by improving the drupal_render() API itself.

added an "after commit" issue