Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#536 Remove duplicate call to contribution summary #13149

Merged
merged 1 commit into from
Nov 23, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

As part of addressing performance issues on the contribution summary tab this first step simply reduces the number of expensive but redundant queries that run

Before

Time spent on running some sql queries twice

After

The 6 queries in question only run once. At volume these are slow queries so this saves around 20
seconds to load a contact with a lot of contributions on a very large database. Even very small databases will get some gain as there are just less queries

We still see the summary
screenshot 2018-11-22 14 56 27

And here

screenshot 2018-11-22 14 57 27

Technical Details

This is an expensive call (we get it at over 10 seconds for high
giving contacts in a large database) and it is quite simply called twice.

Removing these few lines knock out several expensive queries but I have checked
contribution tab, user dashboard, contribution search and advanced search displayed
as contributions and can find no instances where less info is shown.

(in the case of advanced search displayed as contributions it doesn't show before or
after)

Comments

Anything else you would like the reviewer to note

This is an expensive call (we get it at over 10 seconds for high
giving contacts in a large database) and it is quite simply called twice.

Removing these few lines knock out several expensive queries but I have checked
contribution tab, user dashboard, contribution search and advanced search displayed
as contributions and can find no instances where less info is shown.

(in the case of advanced search displayed as contributions it doesn't show before or
after)
@civibot
Copy link

civibot bot commented Nov 22, 2018

(Standard links)

@civibot civibot bot added the master label Nov 22, 2018
@eileenmcnaughton
Copy link
Contributor Author

@pradpnayak @monishdeb @kcristiano @jitendrapurohit I will be interested if any of you can find anywhere adversely affected by this change

@jitendrapurohit
Copy link
Contributor

jitendrapurohit commented Nov 22, 2018

Tested this and verified it on different search forms, contribution dashboard, user dashboard and contact summary pages. The behaviour looks same before and after this change. +1 from my testing results 👍

@colemanw
Copy link
Member

Just to make sure I understand this @eileenmcnaughton - I grepped for a while to try to find where the smarty variable contributionSummary was still being assigned after removing these lines. I see where it is assigned in this file on line 157 but couldn't for the life of me see where that session variable was coming from... until I found this:

$this->_store->set("{$this->_prefix}summary", $summary);

So it's automatically being run from the controller... is that correct?

@eileenmcnaughton
Copy link
Contributor Author

@colemanw yes - it's being run - twice each time it seems!

It's actually an expensive query but my thinking is that if we could call a DIFFERENT function for a singe contact vs a search then we could actually get an optimised query for the single contact. In testing I can get each of the queries in this function from 6 seconds to 6 hundreths of a second with an index hint

@colemanw colemanw merged commit aa4d452 into civicrm:master Nov 23, 2018
@colemanw colemanw deleted the no_double_contribution branch November 23, 2018 00:40
@eileenmcnaughton
Copy link
Contributor Author

As a follow up I've been able to do some testing and this reduces the time to load the contribution tab for our slowest contact from 58-60 seconds to 30 - which is perhaps not a surprising result since we halved the slow queries

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jan 4, 2019
This has been merged upstream - civicrm#13149

I finally got ab benchmarking working well & on this url
civicrm/contact/view/contribution?reset=1&force=1&cid=72&snippet=json
it reduces the average time from 58-60 seconds to 30-31 seconds

This is an expensive call (we get it at over 10 seconds for high
giving contacts in a large database) and it is quite simply called twice.

Removing these few lines knock out several expensive queries but I have checked
contribution tab, user dashboard, contribution search and advanced search displayed
as contributions and can find no instances where less info is shown.

(in the case of advanced search displayed as contributions it doesn't show before or
after)

Bug: T209415

Change-Id: I59a2e9ac42b34a074e698f52de2a06021328f165
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants