Use a static cache in commerce_product_field_extra_fields()

Here's what I did for the performance boost. Any drawbacks?

This is the only one I could find #870292: hook_field_extra_fields() results should be cached by language that was similar but doesn't static cache and doesn't look like it caches... but the title says otherwise.

And this issue says it is cached... but I don't think it is:S
#730308-9: taxonomy_vocabulary_get_names() called on every field_attach_view()

Thanks @rszrama.

@Fabianx I am curious to know where I can read up on the different between the two drupal static caching patterns? They look very similar and @dawehner tried to explain it but didn't quite sink in, the differences and when to use them.

Oh I think it's coming back to me thanks @Fabianx. It's intention is to reduce the # of function calls as well, specifically to drupal_static(). so in this case we wouldn't see 345 new calls to drupal_static(), just one call.

  // Save the function call statically.
  static $drupal_static_fast;
  if (!isset($drupal_static_fast)) {
    // Less calls to this drupal_static() function.
    $drupal_static_fast = &drupal_static(__FUNCTION__);
  }

  // Get our static data.
  $example_data = &$drupal_static_fast; 
  if (!isset($example_data)) {
    // Expensive calculation goes here..
    $example_data = example_data_load();
  } 

So although drupal_static() is cached and fast, not having to call it would be a bit faster if you have many calls. And in this case we have 300+ calls which could be significantly more...

Since I don't see a commit message on this issue, maybe it hasn't been committed and I can roll another patch for that @rszrama?

It can be huge...

function with_static_fast() {
    // Save the function call statically.
  static $drupal_static_fast;
  if (!isset($drupal_static_fast)) {
    // Less calls to this drupal_static() function.
    $drupal_static_fast = &drupal_static(__FUNCTION__);
  }

 // Get our static data.
  $drupal_static = &$drupal_static_fast;
  if (!isset($drupal_static)) {
    // Expensive calculation goes here..
    $drupal_static = example_data_load();
  }
  return $drupal_static;
}

function without_static_fast() {
  $drupal_static = &drupal_static(__FUNCTION__);

   // Get our static data.
  if (!isset($drupal_static)) {
    // Expensive calculation goes here..
    $drupal_static = example_data_load();
  }
  return $drupal_static;
}

function example_data_load() {
  return ['loaded' => 'data'];
}

$START = time();
for ($i = 0; $i < 4000000; ++$i) {
  with_static_fast();
}
$END = time() - $START;
echo "With fast static took $END seconds\n";

$START = time();
for ($i = 0; $i < 4000000; ++$i) {
  with_static_fast();
}
$END = time() - $START;
echo "Without fast static took $END seconds\n";

Run the above in test.php
drush scr test.php
My results were 55 seconds vs 34 seconds.

But with an example even 100 times what we are dealing with here of 347 calls:

$START = microtime(TRUE);
for ($i = 0; $i < 34500; ++$i) {
  with_static_fast();
}
$END = microtime(TRUE) - $START;
echo "With fast static took $END seconds\n";

$START = microtime(TRUE);
for ($i = 0; $i < 34500; ++$i) {
  with_static_fast();
}
$END = microtime(TRUE) - $START;
echo "Without fast static took $END seconds\n";

With fast static took 0.3695080280304 seconds
Without fast static took 0.38044500350952 seconds

And with exactly 345 calls:

With fast static took 0.0026760101318359 seconds
Without fast static took 0.0027229785919189 seconds

So IMO, it's not completely necessary in this case but handy to know if the # calls is > 5000 it could have some nice savings.

@rszrama not sure I follow.

We should be using the &drupal_static. Just not a huge win on the fast drupal_static pattern unless there are a fairly large numbers of calls.

@rszrama I downloaded -dev and it didn't have this patch applied so I'm moving back to NR.

@rszrama ah haha I see now. static -> drupal_static -> drupal_static + static. A bit over engineered yes but I guess like you and @Fabianx said we still can externally clear them out.

Thanks for committing this:) I've heard that bogus auth error happen to a few committers so far on push.

@torgosPizza looking at this patch I think it would be very unlikely to be this patch that caused the performance issue. It just caches the building of the extra form element building and all the extra calls to t() that were happening on every node view mode display.

Maybe the only way it could get worse (and maybe only by a few 5ms per node) is if the static cache was getting cleared all the time. (wild guess).