commerce_discount_date performance issues

I do not understand how the pre-calculation is supposed to work either. I tested it on a clean Kickstart installation and it created 1 row per rule in the database table, and why does it ignore conditions? How is it supposed to work with discounts if it ignores conditions? In my testing on the Kickstart installation I wasn't able to get it to work at all.

And it seems impossible to use pre-calculated prices unless you manually ask for them because there is no way of calling commerce_product_calculate_sell_price() with $precalc = TRUE unless you manually do so or hack the function itself.

This is what I ended up doing. First the hack:

function commerce_product_calculate_sell_price($product, $precalc = FALSE) {
  // Hack to call my own function instead.
  return MYMODULE_commerce_product_calculate_sell_price($product);
}

Then my implementation of the pre-calculation:

function MYMODULE_commerce_product_calculate_sell_price($product) {
  // First create a pseudo product line item that we will pass to Rules.
  $line_item = commerce_product_line_item_new($product);

  // Allow modules to prepare this as necessary.
  drupal_alter('commerce_product_calculate_sell_price_line_item', $line_item);

  // Attempt to fetch a database stored price.
  $module_key = 'MYMODULE';

  $result = db_select('commerce_calculated_price')
    ->fields('commerce_calculated_price', array('amount', 'currency_code', 'data'))
    ->condition('module', 'commerce_product_pricing')
    ->condition('module_key', $module_key)
    ->condition('entity_type', 'commerce_product')
    ->condition('entity_id', $product->product_id)
    ->condition('field_name', 'commerce_price')
    ->execute()
    ->fetchObject();

  // If a pre-calculated price was found.
  if (!empty($result)) {
    // Wrap the line item, swap in the price, and return it.
    $wrapper = entity_metadata_wrapper('commerce_line_item', $line_item);

    $wrapper->commerce_unit_price->amount = $result->amount;
    $wrapper->commerce_unit_price->currency_code = $result->currency_code;

    // Unserialize the saved prices data array and initialize to an empty
    // array if the column was empty.
    $result->data = unserialize($result->data);
    $wrapper->commerce_unit_price->data = !empty($result->data) ? $result->data : array();

    return $wrapper->commerce_unit_price->value();
  }

  // Pass the line item to Rules.
  rules_invoke_event('commerce_product_calculate_sell_price', $line_item);

  // If we found no pre-calculated price save the newly calculated price in the database.
  if (empty($result)) {
    MYMODULE_save_product_price($line_item, $module_key);
  }

  return entity_metadata_wrapper('commerce_line_item', $line_item)->commerce_unit_price->value();
}

function MYMODULE_save_product_price($line_item, $module_key) {
  $product_id = $line_item->commerce_product[LANGUAGE_NONE][0]['product_id'];

  // Build the record of the pre-calculated price and write it.
  $wrapper = entity_metadata_wrapper('commerce_line_item', $line_item);

  if (!is_null($wrapper->commerce_unit_price->value())) {
    $record = array(
      'module' => 'commerce_product_pricing',
      'module_key' => $module_key,
      'entity_type' => 'commerce_product',
      'entity_id' => $product_id,
      'field_name' => 'commerce_price',
      'language' => !empty($product->language) ? $product->language : LANGUAGE_NONE,
      'delta' => 0,
      'amount' => round($wrapper->commerce_unit_price->amount->value()),
      'currency_code' => $wrapper->commerce_unit_price->currency_code->value(),
      'data' => $wrapper->commerce_unit_price->data->value(),
      'created' => time(),
    );

    // Save the price.
    drupal_write_record('commerce_calculated_price', $record);
  }
}

It's the exact same function except it always looks for a pre-calculated price and if it doesn't find one it saves the newly calculated price in the pre-calculation table. The save part of the code is just ripped from the batch operation.

So as you can see it's very much like wrapping the price calculation in cache_get() and cache_set(). It seems to work very well. The pages load quickly and the correct prices are shown.

Edit: I haven't added the node update part yet so it may need to be tweaked some. Also, the comment formatting seems to remove the empty lines within the php tags so the code looks messy. Not my fault. :p

I will investigate further, but that will take some time as I'm quite busy. :)

The root of the problem, as you say, is Commerce Discount in either case. But we're commited to that module at the moment. We've hacked it a lot to make it fit our needs and we will evaluate it further down the line to see if we are to replace it or attempt to patch it.

Things move slow so close to christmas, but I've tested a bit more. :p

We have a few custom rules conditions with node loads etc. for evaluating fields on the node the product belongs to. I thought these could be the culprit so I tried commenting out these, but saw no speed increase at all.

A page with 10 products takes 6-7 seconds to load. If I comment out the rules part of the commerce_product_calculate_sell_price() function and just return the price the same pages takes 600 milliseconds to load.

A kind colleague hooked me up with XHProf and I was able to pinpoint the main source of the problem.

It's the first line in this condition in the commerce_discount_date module:

/**
 * Rules condition: Check discount can be applied.
 */
function commerce_discount_date_condition($discount_name) {
  $wrapper = entity_metadata_wrapper('commerce_discount', $discount_name);
  $time = time();
  return $time >= $wrapper->commerce_discount_date->value->value() && $time <= $wrapper->commerce_discount_date->value2->value();
}

So I guess I'll take it up with the maintainers of that module as all the issues here are caused by the discount module. :)

Edit: I just changed the function that called this one to pass along the wrapper it had already loaded instead of the discount name so commerce_discount_date_condition() wouldn't have to load it again and shaved off almost the entire 6-7 seconds from the page load time. So that one line of code was solely to blame.