Closed (won't fix)
Project:
Drupal core
Version:
7.x-dev
Component:
forms system
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
25 Nov 2005 at 05:30 UTC
Updated:
16 Feb 2010 at 17:54 UTC
Jump to comment: Most recent file
As form API is a little slower than just gluing together HTML chunks, and these forms are present on every page, it could be a small performance boost to cache them. And they never change so they are ideal candidates.
Did some cleanup in search_form.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | form_cache_2.patch | 1.73 KB | chx |
| #10 | form_cache_1.patch | 4.8 KB | chx |
| #8 | form_cache_0.patch | 4.79 KB | chx |
| #6 | form_cache.patch | 6.01 KB | chx |
| #3 | cache_forms_0.patch | 3.81 KB | chx |
Comments
Comment #1
dries commentedBenchmark results needed to back this up.
Comment #2
Steven commentedThe 86400 is obviously supposed to be some timeout, but you are passing it for $headers instead. CACHE_PERMANENT is the timeout value instead (i.E. never).
Comment #3
chx commentedObviously. I really do not know whether I'll be able to show benchmarks, but I'll try some.
Comment #4
moshe weitzman commentedComment #5
chx commentedyes it needs more work because we should not even try to cache when there is a POST -- login won't work!
Comment #6
chx commentedThis patch does caching in form API. The change is as little as possible, it saves the #elements['children'] to cache and loads it back directly. You can specify a cache id but if you don't then form API will generate one for youl
Comment #7
moshe weitzman commentedthis is a pretty interesting patch. i will try to get some benchmarks for it. assigning to me. and marking as 'needs review' for now. its power may be more evident on more complicated forms that are cacheable.
Comment #8
chx commentedrerolled against HEAD
Comment #9
moshe weitzman commented1 hunk failed
Comment #10
chx commentedRerolled.
Comment #11
moshe weitzman commentedi tried to bechmark this but siege/ab were giving too much variation between requests to give good results. i tried our timer API but couldn't persist through all the instrumentation needed to measure this way. hopefully someone else can benchmark this.
Comment #12
killes@www.drop.org commentedI've benchmarked the search form and did not find a statistically significant difference in page execution times.
Comment #13
Tobias Maier commentedI can't comment on the page execution times...
But I think we can use this for the contact module too...
and that we can see real benefits if we cache parts of node/add/X, node/edit/X and modules... (like simple_access)
Comment #14
chx commentedyou provide #cache => TRUE on any form element and it gets cached.
Much better than any patch so far.
Comment #15
moshe weitzman commentedcan we get at least one implementation as well? makes it easier to test and more obvious how to use this.
Comment #16
moshe weitzman commentedcan we get at least one implementation as well? makes it easier to test and more obvious how to use this.
Comment #17
drummI'm guessing something like this will be for 6 at this point.
Comment #18
keith.smith commentedpatch does not apply
# patch -p0 < form_cache_2.patch
patching file includes/form.inc
Hunk #1 FAILED at 263.
Hunk #2 succeeded at 555 with fuzz 2 (offset 27 lines).
Hunk #3 FAILED at 627.
Hunk #4 FAILED at 686.
3 out of 4 hunks FAILED -- saving rejects to file includes/form.inc.rej
Comment #19
panchoMoving this again - this time to the D7 queue.
Comment #20
BioALIEN commentedWow this patch has lasted a few years. As it's marked for D7, does it make sense to also start thinking about caching CCK generated forms?
Comment #21
casey commentedWe do have all kinds of new caching mechanisms for forms in D7. Is this issue fixed by them?
Comment #24
sunI may be mistaken, but I don't think that this patch makes any sense today.