Over in #2123315: Pager does not appear if fields use Views PHP (7.x-1.x) a patch was committed to solve the pager not appearing, so the pager now appears, but performance and memory usage has hit the can.
I haven't been able to understand the intricacies of how the above fix worked, I tried rolling back #2123315: Pager does not appear if fields use Views PHP (7.x-1.x) and tested with #1937364-1: Filter does not respect pagination, it runs through all nodes in a search instead, as I said I don't understand the intricacies, and therefore if this is a valid route to go, but the pager has appeared and performance and memory has improved rather than degraded.
To give you some idea of memory usage and load times.
Before #2123315: Pager does not appear if fields use Views PHP (7.x-1.x) (pager doesn't work):
10 MB + 8 seconds
After #2123315: Pager does not appear if fields use Views PHP (7.x-1.x) (pager works but performance sucks):
535 MB + 15 seconds
With #1937364-1: Filter does not respect pagination, it runs through all nodes in a search instead of #2123315: Pager does not appear if fields use Views PHP (7.x-1.x) (pager works? and performance is great):
10 MB + 7 seconds
Comment | File | Size | Author |
---|---|---|---|
#19 | views_php_memory_fix-2628014-19.patch | 4.7 KB | skylord |
#15 | views_php_memory_fix-2628014-15.patch | 4.7 KB | Suresh Prabhu Parkala |
#8 | views_php_memory_fix-2628014-8.patch | 3.91 KB | rcodina |
#6 | views_php_memory_fix-2628014-6.patch | 4.07 KB | skylord |
#4 | views_php_memory_fix-2628014-4.patch | 4.03 KB | skylord |
Issue fork views_php-2628014
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
bkat CreditAttribution: bkat commentedIn views_php.module, views_php_views_post_build($view) kills the limit/offset in the query and will return all rows thus increasing the page load time.
In my case, it craps out after all memory is consumed by I have a view with 72 pages of 50 items each. The crazy thing is that I don't even have views_php filtering defined for this view. Yes, it kills the query's range even if you just have views_php fields!
Furthermore, I still lose the pager because of
$this->wrapped->total_items = count($this->wrapped->view->result);
and
$this->wrapped->view->result = array_slice($this->wrapped->view->result, $offset, $item_per_page);
int views_php_plugin_pager.inc function updated_wrapped_pager()
PS. I'm on 7.x-2.x-dev but they closed both of the issues on the branch as duplicates of the 7.x-1.x-dev issue
Comment #3
jamesfk CreditAttribution: jamesfk at Website Express Ltd. commentedHi everyone,
Just to confirm we were hitting the memory issues with the latest release of Views PHP and Search API on Drupal 7:
Search API: 7.x-1.25
Views PHP: 7.x-1.0-alpha3
These latest releases had fully fixed pagers and counts, so we encountered no issues with those at all, but unfortunately, on a site with tens of thousands of nodes indexed, the search pages rapidly became unusable with the internal memory / page limits not being respected.
After trying various combinations of modules and patches, we found the following worked perfectly, patch #1937364-1: Filter does not respect pagination, it runs through all nodes in a search run on the alpha 1 release of Views PHP:
https://www.drupal.org/project/views_php/releases/7.x-1.0-alpha1
This is working perfectly (so far) with the latest Search API modules, so may be useful for anyone else hitting this potentially quite popular edge case.
All the best,
James
Comment #4
skylord CreditAttribution: skylord commentedAs pointed out previously the cause of performance degradation is that module forces view to load all items not respecting pagers' settings. It surely needed for sorting and filtering but simple field output is not the case. So, i suggest to enable loading of all items only when it's needed - in filter/sort handlers and fields' click-sort. Here is a patch - works great for me.
Comment #5
MustangGB CreditAttribution: MustangGB commentedCame a cropper of this, so looked into it again, and came to exactly the same conclusion as skylord.
Amazing patch!
My only comment is the help text
This will have huge performance impact as will load full dataset ignoring pager settings
, I suggest it might be clearer to say something likeThis may have a large performance impact as all results will be loaded from the database, and pager limits will only be applied in PHP
.So needs works for the text clarification, but otherwise, I'm liking it a lot.
Comment #6
skylord CreditAttribution: skylord commentedOK, message changed.
Comment #7
MustangGB CreditAttribution: MustangGB commentedTheory is sound, patch looks good, and works as advertised.
Comment #8
rcodina CreditAttribution: rcodina commentedI enclose the patch on #6 adapted to work on branch 7.x-2.x. The only rejected code for patch #6 on branch 7.x-2.x is the following (views_php_plugin_pager.inc.rej):
Comment #11
Liam MorlandI have created an issue fork and merge request with the patch from #6.
Comment #12
joseph.olstadthis project needs a new maintainer , the current maintainers have not been active for a while (look at the project page), I suggest pinging all of them and asking for maintainer priviledges.
Comment #13
Liam MorlandComment #14
Suresh Prabhu Parkala CreditAttribution: Suresh Prabhu Parkala at Specbee commentedComment #15
Suresh Prabhu Parkala CreditAttribution: Suresh Prabhu Parkala at Specbee commentedRe-rolled patch #6. Please review.
Comment #16
Liam MorlandThank you. I would appreciate it if you would update the issue fork.
Comment #17
Liam MorlandI'm moving towards making a release. I would like to get this in. The issue fork needs to be rebased and it needs to be tested back to RTBC. Thanks.
Comment #18
skylord CreditAttribution: skylord commentedReroll from #15 works fine with 7.x-1.0-beta1! Thanks!
Comment #19
skylord CreditAttribution: skylord commentedFound a small bug in #15. Fixed.
Comment #20
Liam MorlandPlease update the issue fork.
Comment #21
joseph.olstadComment #22
MustangGB CreditAttribution: MustangGB commentedThis is a straight up reroll, so marking RTBC again.
Comment #24
Liam MorlandComment #25
MustangGB CreditAttribution: MustangGB commentedHurray!