Hi guys,

Similar to the issue #2057073: Cache form table saves very large records when forms are displayed in a view, which works great for Product Display nodes used in Views, there is a similar issue when using a Commerce Product cart form in a View in which the entire product and its context (as well as additional fields!) are stored in cache_form.

For our use case I have created a View that displays all of a product's variations, based on a node ID. This is helpful for our customers who are given multiple options for purchase, such as in the below screenshot:

Formats

This works by using the Context (NID) and displaying all referenced products in a table. It's pretty low tech. Note the important part: it uses the actual Products, and the Commerce Product "Add to Cart Form" field.

It seems that, while a Product Display node's "Add to Cart" button is not cached when used in views, this is not true when using a Product Entity's "Add to Cart form". This is evident when, in our Devel statistics block, I see this:

INSERT INTO cache_form (cid, serialized, created, expire, data) VALUES ('form_state_form-ZN8qSl8KlARqduCvOYmArjTNseDBRnZD7knEb4pMsXk', '1', '1409768941', '1409790541', 'a:10:{s:10:\"build_info\";a:3:{s:4:\"args\";a:3:{i:0;O:8:\"stdClass\":12:{s:4:\"type\";s:7:\"product\";s:8:\"order_id\";i:0;s:8:\"quantity\";s:1:\"1\";s:4:\"data\";a:1:{s:7:\"context\";a:3:{s:11:\"product_ids\";a:1:{i:0;s:5:\"12991\";}s:19:\"add_to_cart_combine\";b:0;s:4:\"view\";a:4:{s:9:\"view_name\";s:18:\"product_attributes\";s:12:\"display_name\";s:5:\"block\";s:10:\"human_name\";s:39:\"Product types - Available formats block\";s:4:\"page\";N;}}}s:12:\"line_item_id\";N;s:15:\"line_item_label\";s:18:\"Short-ThisIsHormel\";s:7:\"created\";s:0:\"\";s:7:\"changed\";s:0:\"\";s:6:\"is_new\";b:1;s:16:\"commerce_product\";a:1:{s:3:\"und\";a:1:{i:0;a:1:{s:10:\"product_id\";s:5:\"12991\";}}}s:21:\"commerce_display_path\";a:1:{s:3:\"und\";a:1:{i:0;a:1:{s:5:\"value\";s:0:\"\";}}}s:19:\"commerce_unit_price\";a:1:{s:3:\"und\";a:1:{i:0;a:4:{s:6:\"amount\";s:3:\"199\";s:13:\"currency_code\";s:3:\"USD\";s:4:\"data\";a:1:{s:10:\"components\";a:1:{i:0;a:3:{s:4:\"name\";s:10:\"base_price\";s:5:\"price\";a:3:{s:6:\"amount\";s:3:\"199\";s:13:\"currency_code\";s:3:\"USD\";s:4:\"data\";a:1:{s:10:\"components\";a:0:{}}}s:8:\"included\";b:1;}}}s:8:\"original\";a:3:{s:6:\"amount\";s:3:\"199\";s:13:\"currency_code\";s:3:\"USD\";s:4:\"data\";a:1:{s:10:\"components\";a:0:{}}}}}}}i:1;i:0;i:2;a:0:{}}s:7:\"form_id\";s:36:\"commerce_cart_add_to_cart_form_12991\";s:12:\"base_form_id\";s:30:\"commerce_cart_add_to_cart_form\";}s:10:\"programmed\";b:0;s:30:\"programmed_bypass_access_check\";b:1;s:5:\"cache\";b:1;s:7:\"context\";a:0:{}s:9:\"line_item\";r:4;s:15:\"default_product\";O:8:\"stdClass\":26:{s:11:\"revision_id\";s:5:\"12906\";s:3:\"sku\";s:18:\"Short-ThisIsHormel\";s:5:\"title\";s:14:\"This is Hormel\";s:12:\"revision_uid\";s:1:\"1\";s:6:\"status\";s:1:\"1\";s:3:\"log\";s:0:\"\";s:18:\"revision_timestamp\";s:10:\"1393613853\";s:4:\"data\";b:0;s:10:\"product_id\";s:5:\"12991\";s:4:\"type\";s:5:\"video\";s:8:\"language\";s:3:\"und\";s:3:\"uid\";s:1:\"1\";s:7:\"created\";s:10:\"1393613853\";s:7:\"changed\";s:10:\"1394497396\";s:14:\"commerce_price\";a:1:{s:3:\"und\";a:1:{i:0;a:4:{s:6:\"amount\";s:3:\"199\";s:13:\"currency_code\";s:3:\"USD\";s:4:\"data\";a:1:{s:10:\"components\";a:1:{i:0;a:3:{s:4:\"name\";s:10:\"base_price\";s:5:\"price\";a:3:{s:6:\"amount\";s:3:\"199\";s:13:\"currency_code\";s:3:\"USD\";s:4:\"data\";a:1:{s:10:\"components\";a:0:{}}}s:8:\"included\";b:1;}}}s:8:\"original\";a:3:{s:6:\"amount\";s:3:\"199\";s:13:\"currency_code\";s:3:\"USD\";s:4:\"data\";a:1:{s:10:\"components\";a:0:{}}}}}}s:16:\"field_legacy_nid\";a:1:{s:3:\"und\";a:1:{i:0;a:3:{s:5:\"value\";s:7:\"3325286\";s:6:\"format\";N;s:10:\"safe_value\";s:7:\"3325286\";}}}s:12:\"field_poster\";a:1:{s:3:\"und\";a:1:{i:0;a:13:{s:3:\"fid\";s:6:\"123876\";s:3:\"uid\";s:1:\"1\";s:8:\"filename\";s:22:\"ThisIsHormelPoster.jpg\";s:3:\"uri\";s:45:\"public://posters/video/ThisIsHormelPoster.jpg\";s:8:\"filemime\";s:10:\"image/jpeg\";s:8:\"filesize\";s:5:\"67972\";s:6:\"status\";s:1:\"1\";s:9:\"timestamp\";s:10:\"1403314940\";s:11:\"rdf_mapping\";a:0:{}s:3:\"alt\";s:0:\"\";s:5:\"title\";s:0:\"\";s:5:\"width\";s:3:\"450\";s:6:\"height\";s:3:\"600\";}}}s:19:\"field_vidly_feature\";a:1:{s:3:\"und\";a:1:{i:0;a:3:{s:5:\"value\";s:6:\"3g2g5d\";s:6:\"format\";N;s:10:\"safe_value\";s:6:\"3g2g5d\";}}}s:21:\"commerce_license_type\";a:1:{s:3:\"und\";a:1:{i:0;a:1:{s:5:\"value\";s:4:\"file\";}}}s:25:\"commerce_license_duration\";a:1:{s:3:\"und\";a:1:{i:0;a:1:{s:5:\"value\";s:1:\"0\";}}}s:13:\"commerce_file\";a:1:{s:3:\"und\";a:4:{i:0;a:11:{s:3:\"fid\";s:5:\"37861\";s:3:\"uid\";s:1:\"1\";s:8:\"filename\";s:23:\"ThisIsHormel_legacy.mp4\";s:3:\"uri\";s:35:\"s3://shorts/ThisIsHormel_legacy.mp4\";s:8:\"filemime\";s:9:\"video/mp4\";s:8:\"filesize\";s:9:\"127424732\";s:6:\"status\";s:1:\"1\";s:9:\"timestamp\";s:10:\"1403045340\";s:11:\"rdf_mapping\";a:0:{}s:7:\"display\";s:1:\"1\";s:11:\"description\";s:14:\"Legacy devices\";}i:1;a:11:{s:3:\"fid\";s:5:\"37866\";s:3:\"uid\";s:1:\"1\";s:8:\"filename\";s:20:\"ThisIsHormel_low.mp4\";s:3:\"uri\";s:32:\"s3://shorts/ThisIsHormel_low.mp4\";s:8:\"filemime\";s:9:\"video/mp4\";s:8:\"filesize\";s:9:\"205556311\";s:6:\"status\";s:1:\"1\";s:9:\"timestamp\";s:10:\"1403045340\";s:11:\"rdf_mapping\";a:0:{}s:7:\"display\";s:1:\"1\";s:11:\"description\";s:3:\"Low\";}i:2;a:11:{s:3:\"fid\";s:5:\"37871\";s:3:\"uid\";s:1:\"1\";s:8:\"filename\";s:20:\"ThisIsHormel_med.mp4\";s:3:\"uri\";s:32:\"s3://shorts/ThisIsHormel_med.mp4\";s:8:\"filemime\";s:9:\"video/mp4\";s:8:\"filesize\";s:9:\"374362097\";s:6:\"status\";s:1:\"1\";s:9:\"timestamp\";s:10:\"1403045340\";s:11:\"rdf_mapping\";a:0:{}s:7:\"display\";s:1:\"1\";s:11:\"description\";s:6:\"Medium\";}i:3;a:11:{s:3:\"fid\";s:5:\"37856\";s:3:\"uid\";s:1:\"1\";s:8:\"filename\";s:23:\"ThisIsHormel_highTV.mp4\";s:3:\"uri\";s:35:\"s3://shorts/ThisIsHormel_highTV.mp4\";s:8:\"filemime\";s:9:\"video/mp4\";s:8:\"filesize\";s:9:\"599633637\";s:6:\"status\";s:1:\"1\";s:9:\"timestamp\";s:10:\"1403045340\";s:11:\"rdf_mapping\";a:0:{}s:7:\"display\";s:1:\"1\";s:11:\"description\";s:7:\"High-TV\";}}}s:22:\"field_publication_date\";a:1:{s:3:\"und\";a:1:{i:0;a:4:{s:5:\"value\";s:19:\"2014-02-28 08:00:00\";s:8:\"timezone\";s:19:\"America/Los_Angeles\";s:11:\"timezone_db\";s:19:\"America/Los_Angeles\";s:9:\"date_type\";s:8:\"datetime\";}}}s:18:\"field_release_date\";a:0:{}s:18:\"field_product_icon\";a:0:{}s:11:\"rdf_mapping\";a:0:{}s:15:\"display_context\";a:5:{s:11:\"entity_type\";s:4:\"node\";s:9:\"entity_id\";s:7:\"3341376\";s:6:\"entity\";O:8:\"stdClass\":36:{s:3:\"vid\";s:5:\"16391\";s:3:\"uid\";s:1:\"1\";s:5:\"title\";s:14:\"This is Hormel\";s:3:\"log\";s:0:\"\";s:6:\"status\";s:1:\"1\";s:7:\"comment\";s:1:\"0\";s:7:\"promote\";s:1:\"0\";s:6:\"sticky\";s:1:\"0\";s:3:\"nid\";s:7:\"3341376\";s:4:\"type\";s:4:\"riff\";s:8:\"language\";s:3:\"und\";s:7:\"created\";s:10:\"1403386099\";s:7:\"changed\";s:10:\"1403386099\";s:4:\"tnid\";s:1:\"0\";s:9:\"translate\";s:1:\"0\";s:18:\"revision_timestamp\";s:10:\"1403386099\";s:12:\"revision_uid\";s:1:\"1\";s:16:\"field_collection\";a:0:{}s:17:\"field_description\";a:1:{s:3:\"und\";a:1:{i:0;a:3:{s:5:\"value\";s:1588:\"
This Is Hormel is a short in the grand tradition of David and Hazel and Setting Up A Room, where you look at the time remaining on your media player and think “Clearly that has to be a mistake.” But no! It’s a half hour long Hormel infomercial masquerading as an educational short that must be seen to be believed!
... 
(snipped because it's so long, the Issue Form could not handle it.)

Is there any chance the "do not cache entity context" functionality from the related Issue could be ported to the Commerce Product: Add to Cart Form Views handler? I believe it would significantly help our performance.

Thanks!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

torgosPizza’s picture

torgosPizza’s picture

After doing some digging it looks like this may be a Views issue, as bojanz has dug into this within this past year.

#2059555: View needlessly stored in form state causes huge cache_form entries

So it seems a workaround for this use case may still be warranted.

torgosPizza’s picture

Continuing to dig, and it looks like $form_state['default_product'] is the culprit. This object is created in commerce_cart_add_to_cart_form() and passed to the form builder functions, which then caches the entire thing in cache_set_form.

Is $default_product used in a lot of places, and would removing it at the end of the cart form-building process break a lot of things? I'm examining the code now and it seems like a lot of this code could be refactored into a system that only passes relevant data rather than an entire product object.

The main problem I see with the current approach is that the $form_state['default_product'] object contains every field attached to a product, which in our case can be awfully large. Those inserts are one of the main things currently causing performance bottlenecks with every INSERT into cache_form( almost 3 seconds out of a 5-second Views page load).

rszrama’s picture

Title: Do not cache entire product when Commerce Product Cart Forms are used in Views » Remove the default product entity from the form state just prior to caching
Category: Bug report » Task
Issue tags: +scalability

Erik and I discussed this in IRC, and it seems like a workable plan of action would be to add an #after_build callback to the Add to Cart form that removed $form_state['default_product'] to prevent it from being cached. The idea is to preserve its presence for any other module that depends on it in a form alter but to not worry about retaining it in the cache since the variable is actually set during the form build process anyways.

See this recent commit to add an #after_build callback to the checkout form for an example:

http://cgit.drupalcode.org/commerce/commit/?id=bea358f

torgosPizza’s picture

Status: Active » Needs review
FileSize
1.04 KB

Thanks Ryan! Here is a patch which does just that.

Would love any and all people experiencing this issue to test this patch out. I will try to do some benchmarking through Devel and XHProf.

torgosPizza’s picture

Testing on my local site, the default_product object is removed from the $data array that gets serialized and cached.

Totally empirical right now but on our test server, the time to load the catalog Views page is cut in half. It went from about 4-5 seconds to about 2.5. Awesome gains!

Status: Needs review » Needs work

The last submitted patch, 5: commerce-add-to-cart_after-build.patch, failed testing.

torgosPizza’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

Here is my second attempt at a patch which addresses the exceptions from the previous test:

- Adds an #after_build to commerce_cart_add_to_cart_form();
- Adds the parameter default_product_id after it unsets default_product from the $form_state array;
- In commerce_cart_add_to_cart_form_attributes_refresh() it loads the product entity based on the default_product_id and repopulates $form_state['default_product'] with that object.

Hopefully this will pass the tests and at the same time prevent any Contribs using Cart ajax callbacks from breaking.

Status: Needs review » Needs work

The last submitted patch, 8: commerce_cart-after_build-2332333-8.patch, failed testing.

torgosPizza’s picture

Okay, so all of those failures are happening in the #after_build callback, presumably because the form has now been cached without $form_state['default_product']. I guess I should check for the existence of it first with !empty() / !isset()?

torgosPizza’s picture

Status: Needs work » Needs review
FileSize
1.71 KB

Okay, my last try for today... wrapped the after_build code around an isset() condition. We'll see how this one goes.

torgosPizza’s picture

Now that it's passed TestBot I'd love to get others testing it with their contrib, if at all possible. Thank you!

torgosPizza’s picture

Just noticed an undefined function error, as in my patch I call "commerce_load()". . this should be commerce_product_load. I'll post another patch soon.

bojanz’s picture

Status: Needs review » Needs work

According to #13.

torgosPizza’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.72 KB

Thanks for the reminder! New patch attached.

Been using this in production with no problems.

bojanz’s picture

Status: Reviewed & tested by the community » Fixed

Committed, thank you.

  • bojanz committed 40a7e55 on 7.x-1.x authored by torgosPizza
    Issue #2332333 by torgosPizza: Remove the default product entity from...

Status: Fixed » Needs work

The last submitted patch, 15: commerce_cart-after_build-2332333-15.patch, failed testing.

bojanz’s picture

Status: Needs work » Fixed
torgosPizza’s picture

Awesome, thanks!

Status: Fixed » Closed (fixed)

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