After digging into webform module I have found, that it spending a lot of memory and time when trying to get all submissions (tested on prod where are ~20K submissions)

Peak memory usage was 292.55 MB [317.47 sec, 41.89 MB]

Now memory decreased to ~140MB and time to 27 seconds for webform with ~20000 submissions.

So it is >10x speedup.

  • I have used subquery ability for retrieving submissions.
  • Also added static cache for function, that was called multiple times.
  • Fixed trouble, when there are FALSE submissions within database and dblog dying from Notices.
  • Fixed submissions counter wrong count displaying bug

needs review

Comments

podarok’s picture

Issue summary: View changes
podarok’s picture

Issue summary: View changes
podarok’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 10x_speedup.patch, failed testing.

danchadwick’s picture

Version: 7.x-3.x-dev » 7.x-4.3
Category: Bug report » Task
Priority: Major » Normal

@podarok - Thank you very much for your efforts at optimizing webform. In reading your patch, I have a few comments:

  1. This patch would only be considered for the 7.x-4.x and 8.x branches. A back-port to 7.x-3.x is possible, since we have a new maintainer for that branch.
  2. The patch doesn't apply as I believe it isn't made at the right directory level.
  3. Your patch spans several completely unrelated optimizations. I'd like to see each presented individually. I'd also like to see some quantitative data (workload, timing results with and without the optimization). Obviously we only want to apply optimizations that are effective.
  4. The optimization to webform_component_feature, if effective, looks good but could be more easily made with the static design pattern. There would be no need for another module to reset the cache at runtime, and the features could simply be stored in one static array. This mean that webform_components() would be called exactly once, rather than once for each combination of $type and $feature, resulting in a simpler and more effective optimization.
  5. The optimization to webform_results_export / $submission_information is optimizing an array of only 3 value and if it makes even a nanosecond of difference then my head will explode at the awfulness of PHP. The foreach should call array_values() exactly once.
  6. There are some line spacing and whitespace changes that should be removed. They just complicate the patch.
  7. I haven't studied the pager code carefully, so my comments are just things that I'm wondering. Are the table references 'ws.' actually needed to resolve ambiguity? If not, they should be removed as a stylistic difference.
  8. I'm surprise to see the call to fetchAll(). I would have thought that this would require significantly more memory. Have you profiled this change individually?

Please don't be deterred. I am very enthusiastic about committing effective optimizations. Also, I've adjusted the issue metadata to correspond to Issue Queue Handbook standards.

podarok’s picture

Please, add me to co-maintainers, I'll support 3x branch, because we have hi-load project that looks like never be updated to 4x branch
I'll reroll this patch asap

danchadwick’s picture

@podarok -- As luck would have it, user torotil has just volunteered to be webform's new 7.x-3.x branch maintainer. The good news is that since both you and he are interested in maintaining this branch, you can review each other's work. He has another issue in the queue that I'm sure he'd like eyes on (#1332820: Parts of nested tokens (in fieldsets) printed when left empty). And I discussed this issue with him and he's ready to help review patches after the review in #5 is attended to.

I'm happy to see the enthusiasm for 7.x-3.x and for optimization.

  • DanChadwick committed 5d8e8c4 on 7.x-4.x
    Issue #2442917 by podarok: Minor optimization to webform_components()...

  • DanChadwick committed f08272c on 8.x-4.x
    Issue #2442917 by podarok: Minor optimization to webform_components()...
danchadwick’s picture

Version: 7.x-4.3 » 7.x-4.x-dev
Status: Needs work » Fixed
Issue tags: -Performance
StatusFileSize
new1.78 KB

No activity in a month, so I went through this patch again in the context of the active branches.

1) I see static caching added to webform_component_feature(). It doesn't do much, except call webform_components(), which is itself cached. I'm unconvinced of a big win here, but i didn't do any testing to confirm this. I'd be very happy to be proved wrong.

I did make a tiny optimization to webform_components, eliminating a array_diff_key. With only a dozen component types, I doubt it helps much, but I think it makes the code clearer.

2) There is a probably big win optimization to webform_get_submissions when a header is used. Due to changes in the 4.x branch, webform_get_submissions is no longer called for the submissions or results table, as these are done with views. The 8.x branch should remove the header option probably.

3) I don't understand how the change to webform_get_submission_count would help. I didn't check the 3.x branch, but the 4.x branch has indices on webform_submissions to make queries with nid and optionally uid very fast. I did see a line of left-over code, which I deleted.

Committed to 7.x-4.x and 8.x.

Alas the big win is an optimization in code no longer used in the current branch.

danchadwick’s picture

My thanks to @podarok for the webform_get_submissions optimization. It will end up in webform as part of the optimization and fixes for batch download. See the related issue.

Status: Fixed » Closed (fixed)

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