views_handler_field_field::set_items() does this:
$display = array(
'type' => $this->options['type'],
'settings' => $this->options['settings'],
'label' => 'hidden',
// Pass the View object in the display so that fields can act on it.
'views_view' => $this->view,
'views_field' => $this,
'views_row_id' => $row_id,
);
If the field formatter outputs a form (commerce_add_to_cart_form, for instance), $display gets stored in form_state.
This means that each row stores the entire view in form state, leading to entries in the db that are 8-9mb each in certain cases (related issue: #2057073: Cache form table saves very large records when forms are displayed in a view).
Those three params were added for the editablefields module, I remember that.
We can't remove them without breaking the module (which is quite broken at the moment anyway) and any others.
However, what we can do is introduce a minor BC break, replacing views_view with views_view_name.
This allows any module to load the view when needed, while keeping it out of form state, eliminating most of the baggage.
Comment | File | Size | Author |
---|---|---|---|
#25 | 2059555-views-25.patch | 4.02 KB | garphy |
#20 | interdiff-2059555-16-20.txt | 1.59 KB | michfuer |
#20 | 2059555-views-20.patch | 4.23 KB | michfuer |
#16 | interdiff.txt | 976 bytes | tim.plunkett |
#16 | 2059555-views-16.patch | 4.22 KB | tim.plunkett |
Comments
Comment #1
bojanz CreditAttribution: bojanz commentedEasy.
Comment #2
bojanz CreditAttribution: bojanz commentedOf course, I could add a workaround to Commerce, unsetting parts of $context that get put in form state, but this seems like a sensible change anyway.
Comment #3
bojanz CreditAttribution: bojanz commentedAdded a workaround in #2057073: Cache form table saves very large records when forms are displayed in a view instead, since views_field needs to be unset as well, which is not an option because of the BC break. Also, the situation is pretty much limited to Commerce.
Comment #4
xjmSince this is pretty nasty when it manifests itself on any exposed filter with an unreasonably large list, we could probably do something to be a little more defensive about what we stuff in the cache table. The correct solution is to use a sensible form element for the situation, but it's totally conceivable for a field's values to evolve a way the view builder doesn't anticipate and make the site melt before someone tracks down the problem. So reopening for some more discussion.
I also think I've run into problems in the Views UI simply trying to configure a field that has lots of potential values (when working on d.o features for example).
Comment #5
Darth_Beholder CreditAttribution: Darth_Beholder commentedI have a view with editable fields (from editablefields module).
Every time i refresh my view page, cache_form grows by 1,5-2mb.
After applying this patch, it grows rapidly, by 9-10mb at each refresh.
I check this twice. Any suggestions?
Comment #6
maximn CreditAttribution: maximn commentedthere is still a huge problem with cache_form and views. i have ubercart installed and add to cart forms inside a view and my cache_form table grows like crazy - 1.5-2 gigabytes in couple hours, and this is just a test site with no real traffic.
i applyed the path with no change in behaviour at all, so i decided to make some further investigation on this. turn out to be, that the whole 'view' objet is cached on each form caching, that's why cache table grows so fast. in my case the problem was in file views_plugin_row_node_view.inc::render()
as you can see, whole view object is put into $node and later $node is cached by form_cache mechanism. i'm not sure why is that made like that, so i just made small hook in hook_form_alter to unset this property before it is cached.
i'm sure there is some better workaround, but this worked fine for me
Comment #7
rvillan CreditAttribution: rvillan commentedHere is the patch updated for 3.x-dev.
Comment #8
joelpittetComment #9
dawehnerIt would be great if we could not break BC by default but instead have some checkbox (just on 7.x-3.x but not on D8) which removes the view from it.
Comment #10
joelpittetJust throwing this out there, could we just not cache the form?
I had a bit of success with just doing and
['#after_build'][] = 'MODULE_after_build'
callback forviews_form_commerce_cart_default
By setting this in the callback:
My huge cache_form serialize/unserialize just went away, *POOF*. And things still *seemed* to continue to work... but I guess this would need more tests but let me know if you know of a scenario which this would be a bad idea?
FYI, i'm of the opinion form and form_state are not caches in the first place but if we can remove them from views successfully that may be a big win... Here is an experiment inline with that to those who are interested #2183275: Use cache for $form, kv for $form_state
I'm using the patch in #7 successfully but still have large form and form_state because
views_handler_field_field::post_execute()
loads the entire entity on the results special field called_field_data
and the entireView
still gets added via the$form_state['build_info']['args'][0]
Comment #11
joelpittetA better docs reference #689084-12: Document $form_state['cache'] for
#no_cache
. It seems to have some uses with#ajax
forms and multistep forms. So in my cart example above it would only be fine if I didn't turn ajax on that view I'd assume, which I'd like to do... later;)Comment #12
tim.plunkettAs an experiment, backporting (parts of) #2252763: Views exposed filter form causes enormous form state cache entries.
Comment #13
tim.plunkettThis is the key line from the D8 patch.
Comment #14
tim.plunkettComment #16
tim.plunkettD8 has solved a lot of problems through ViewExecutable and PluginCollection
Comment #18
shawn_smiley CreditAttribution: shawn_smiley at Achieve Internet commentedNote that with the patch in #16 that we see a large number of unserialization errors: "Erroneous data format for unserializing 'view' cache.inc:439".
I haven't had a chance to track down the source of the issue though.
Comment #19
gumrol CreditAttribution: gumrol as a volunteer commentedFor anyone using Ubercart and experiencing the same issue - the proposed solution in #10 by @joelpittet works like a charm, just check the caveats he mentions in #11.
The below is a workaround, and won't work in all cases. In my case, I have no "add to cart" forms on my product listing pages. My cache_form table grows in Gigs per day caching these forms needlessly, so a drastic measure is needed:
Use a
hook_form_alter
(dynamic in Ubercart, so you need to make provision for this) to register your'#after_build'
callback, only when the key 'view' is available on the node entityisset($form['node']['#value']->view)
. (You can see why this is not an ideal solution). Your callback function will contain exactly what @joelpittet puts forward in #10 - thanks Joel!cache_form on my site now stays trimmed with the cron and is in the Mb size range instead of Gb.
Comment #20
michfuer CreditAttribution: michfuer at Mediacurrent commentedUsing the patch in #16, I'm seeing some double digit reductions (~20%) in the size of form_state_form-* records `data` column.
Running into some errors, so re-rolled.
For the "Erroneous data format for unserializing 'view' cache.inc:449" errors I had to manually truncate the `cache_views` table. Relevant info from https://github.com/doctrine/doctrine2/issues/3897
Comment #22
DamienMcKennaI can confirm the test failures locally. I'll try to look at them soon.
Comment #23
michfuer CreditAttribution: michfuer at Mediacurrent commentedI'm not having any luck reproducing the test failures locally. I've got the latest 7.x dev version of Drupal core, and 7.x-3.x-dev of Views with the patch in #20 applied. I see a few exceptions (most related to this issue https://www.drupal.org/node/2331209), but no failures.
Any advice on reproducing the test bot failures locally is much appreciated!
Comment #24
DamienMcKenna@michfuer: I reran the tests for the patch and don't see any exceptions without the patch, but all of those exceptions show when the patch is enabled. The exceptions need to be fixed, we won't commit the change as-is.
Comment #25
garphy CreditAttribution: garphy at ICI LA LUNE commentedRerolled #20