Problem/Motivation

When working on #1513198: Allow Views export of all style settings I noticed my views_plugin_style is called many times. This is a performance killer and maybe a bug.

Steps to reproduce:

1. Probe your option_definition with ie a dsm()
2. When views a view you notice 3 dsm outputs
3. When editing a view you notice 15 calls

You can also probe the option_definition function like this

  function option_definition() {
    static $stack;
    if (!isset($stack)) $stack = array();

... normal code
    
    $trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT);
    $substack = &$stack;
    foreach ($trace as $caller) {
      $id = '';
      if (isset($caller['class'])) {
        $id = $caller['class'] . "::";
      }
      $id .= $caller['function'];
      if (!isset($substack[$id])) {
        $substack[$id] = array();
      }
      $substack = &$substack[$id];
    }
    dsm($stack, __CLASS__ . "::" . __METHOD__);
    
    return $options;

This shows a call stack like

... (Array, 1 element)

    views_plugin_style_graphapi::option_definition (Array, 3 elements)
        views_object::set_default_options (Array, 1 element)
            views_object::construct (Array, 1 element)
                _views_create_handler (Array, 1 element)
                    views_get_plugin (Array, 2 elements)
                        views_plugin_display::get_plugin (Array, 2 elements)
                            views_plugin_display::uses_fields (Array, 1 element)
                            views_plugin_display::validate (Array, 2 elements)
                                view::validate (Array, 1 element)
                                views_plugin_display_page::validate (Array, 1 element)
                        view::init_style (Array, 1 element)
        views_object::unpack_options (Array, 1 element)
            views_plugin_style::init (Array, 2 elements)
                views_plugin_display::get_plugin (Array, 2 elements)
                    views_plugin_display::uses_fields (Array, 1 element)
                        views_plugin_display::validate (Array, 2 elements)
                            view::validate (Array, 1 element)
                            views_plugin_display_page::validate (Array, 1 element)
                    views_plugin_display::validate (Array, 2 elements)
                        view::validate (Array, 1 element)
                        views_plugin_display_page::validate (Array, 1 element)
                view::init_style (Array, 1 element)
                    view::build (Array, 1 element)
        views_plugin_style_graphapi::theme_functions (Array, 1 element)
            views_plugin_style_graphapi::render (Array, 1 element)

which indicates we can improve this probably.

Proposed resolution

1. Cache the value if possible
2. Document whether the views (plugin) extension developer can cache his results.

CommentFileSizeAuthor
#2 views-cache-option-definition-1532216-2.patch2.26 KBclemens.tolboom

Comments

dawehner’s picture

dawehner
Primary language German
commented

At least for the normal definitions of option_definition it's a simple php array so it's performance is hard to beat,
so i guess for most cases it's not really worth to cache, everything else which happens after this, for example unpack_options is much harder, so it's cached, at least for displays.

I guess the easiest solution would be to cache your results in the style object.
You actually had the first appearance with problems of this function, but maybe other people didn't saw it before.

I saw on graphapi that you call option_definition in theme_functions(), without any special reason

    $options = $this->options + $this->option_definition();
    $hook =  'views_graphapi_style_graphapi';
    return views_theme_functions($hook, $this->view, $this->display);

Actually i guess you could use the default implementation as well, it should work for you as well, because you can set the theme function in your hook_views_plugins.

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new views-cache-option-definition-1532216-2.patch2.26 KB

Probing a little differently and adjusting base.inc to cache option_definition I can reduce the calls by a factor 2.

  function option_definition() {
    static $stack;
    if (!isset($stack)) $stack = array();

...

    $obj_id = spl_object_hash($this);
    if (!isset($stack[$obj_id])) {
      $stack[$obj_id] = array();
    }
    $substack = &$stack[$obj_id];

    $trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT);
    foreach ($trace as $caller) {
      $id = '';
      if (isset($caller['class'])) {
        $id = $caller['class'] . "::";
      }
      $id .= $caller['function'];
      if (!isset($substack[$id])) {
        $substack[$id] = array();
      }
      $substack = &$substack[$id];
    }
    
    dsm($stack, __CLASS__ . "::" . __METHOD__);
    
    return $options;
  }

What I (guess I) learned is

- each display has a plugin_style object: loading the views edit page needs two in my case.
- why are there 5 more objects needed when editing a view? Is it the preview?
- viewing the result on admin/modules/graph has (now) just 1 call as expected.

dawehner’s picture

dawehner
Primary language German
commented
Assigned: Unassigned » merlinofchaos

Assign the issue to merlinofchaos

Personally i think this introduces more level of complexity for things which doesn't matter in the most use cases and can be fixed in custom code.

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented

Hmmm ... I'm not in favor of caching my option_definitions. I think this is a structural views 'problem' with a partly? solvable solution ;)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Bumping, just to see if merlinofchaos agrees with #3 or not.

drewish’s picture

drewish commented

Maybe I'm missing something but I don't see why the caching can't happen in set_default_options(). Why does there need to be a _option_definition() function?

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented

@drewish does that solve the multiple calls?

As the 'stacktrace' from the summary shows my graph API ::option_definitions() is called from so many places for many times. (see numbers below)

    views_plugin_style_graphapi::option_definition (Array, 3 elements)
        views_object::set_default_options (Array, 1 element)
            views_object::construct (Array, 1 element)
                _views_create_handler (Array, 1 element)
                    views_get_plugin (Array, 2 elements)
                        views_plugin_display::get_plugin (Array, 2 elements)
                            views_plugin_display::uses_fields (Array, 1 element)
                            views_plugin_display::validate (Array, 2 elements)
     1                           view::validate (Array, 1 element)
     2                           views_plugin_display_page::validate (Array, 1 element)
     3                   view::init_style (Array, 1 element)
        views_object::unpack_options (Array, 1 element)
            views_plugin_style::init (Array, 2 elements)
                views_plugin_display::get_plugin (Array, 2 elements)
                    views_plugin_display::uses_fields (Array, 1 element)
                        views_plugin_display::validate (Array, 2 elements)
     4                       view::validate (Array, 1 element)
     5                       views_plugin_display_page::validate (Array, 1 element)
                    views_plugin_display::validate (Array, 2 elements)
     6                   view::validate (Array, 1 element)
     7                   views_plugin_display_page::validate (Array, 1 element)
                view::init_style (Array, 1 element)
     8               view::build (Array, 1 element)
        views_plugin_style_graphapi::theme_functions (Array, 1 element)
     9      views_plugin_style_graphapi::render (Array, 1 element)

Thus adding a private method for getting the settings is way faster.
Remember views_plugin_style_graphapi::option_definition is calling implementers too many times :/

Did this makes sense?

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented
Issue summary: View changes

Fixed a array dump(er) a little.

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented

Still working on Graph API issue #1513198: Allow Views export of all style settings. Debugging is hard due to these 9 calls :(

I guess we should call it a day for this issue. I have to upgrade Graph API to views-8.x anyway.

chris matthews’s picture

chris matthews commented
Assigned: merlinofchaos » Unassigned
Status: Needs review » Closed (outdated)

The 7 year old patch in #2 to base.inc does not apply to the latest views 7.x-3.x-dev, but closing the issue as outdated.

Checking patch includes/base.inc...
error: while searching for:
  var $options = array();

  /**
   * The top object of a view.
   *
   * @var view

error: patch failed: includes/base.inc:15
error: includes/base.inc: patch does not apply