Optimize webroot/tiki-editpage.php and associated code

VERIFIED FIXED in 1.0

Status

support.mozilla.org
Knowledge Base Software
--
blocker
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: ecooper, Assigned: ecooper)

Tracking

({perf})

unspecified
x86
All
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: tiki_bug, tiki_upstreamed, URL)

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
It seems that our timeout troubles on staging are being caused by the code in tiki-editpage.php.

We're seeing out of control execution times:
[Wed Mar 18 23:00:01 2009] [error] [client 10.2.81.4] PHP Fatal error:  Maximum execution time of 30 seconds exceeded in /data/www/support-stage.mozilla.com/webroot/setup_smarty.php on line 107, referer: https://support-stage.mozilla.org/tiki-editpage.php?page=jkdlsf&quickedit=Edit

And high memory usage:
PHP Fatal error: Allowed memory size of 33554432 bytes exhausted (tried to
allocate 40961 bytes)

You can also view the timeout bug here for other bits and pieces <https://bugzilla.mozilla.org/show_bug.cgi?id=480415>.

I'm assigning this to me, but if someone finds time on their hands, we can tag team this.
Keywords: perf
OS: Linux → All
I'm not sure about time, but I'd be willing to help out. If you want, you can try to sub-assign a specific piece of code to me once you get going.

Updated

9 years ago
Target Milestone: 1.1 → 1.0
Eric, how's this? I could spend some time on it, mostly tomorrow though. If you've got some input or want me to take it, let me know!
(Assignee)

Comment 3

9 years ago
I see this as a general 'benchmark and see' sort of bug...trim the fat, if you will. So, find the slow, wasteful parts and make them faster. ;)

That being said, I haven't had a chance to do any benchmarks on it yet. That'd be a good start. Or if you want to see if the function at setup_smarty:107 can be optimized, that'd be good too.

Comment 4

9 years ago
Paul, Eric - this is the most important bug still open on 1.0.  Without it we can't edit the l10n dashboard.  Please drop everything else and work on this one.
Severity: enhancement → blocker
(Assignee)

Comment 5

9 years ago
Waiting for input from Nelson regarding the poll data fetched in webroot/tiki-editpage.php that's only displayed for non-KB categories.

As I've mentioned, I don't understand why we would need it. Getting rid of that seems to get us out of the red for that page. 

For additional reference: see webroot/tiki-editpage.php:968-995.

Comment 6

9 years ago
Eric, can you post a patch for testing?

Comment 7

9 years ago
PS this is blocking 1.0 since we can't edit the l10n dashboard.

Comment 8

9 years ago
For sumo, we have custom behavior in poll_categorize.php that creates CSAT polls, it is commented as

// for Mozilla, wiki pages in the knowledge base has polls of certain questions

This should not depend on there being any polls set in tiki-editpage.php. Your recommended disabling of handling poll amendment from tiki-editpage.php seems ok.



(In reply to comment #5)
> Waiting for input from Nelson regarding the poll data fetched in
> webroot/tiki-editpage.php that's only displayed for non-KB categories.
> 
> As I've mentioned, I don't understand why we would need it. Getting rid of that
> seems to get us out of the red for that page. 
> 
> For additional reference: see webroot/tiki-editpage.php:968-995.
(Assignee)

Comment 9

9 years ago
Created attachment 369565 [details] [diff] [review]
Get rid of poll processing in tiki-editpage.php

So this patches gets rid of the fetching of poll info that occurs during every load of tiki-editpage.php.

Some notes about the patch: 
* The diff of tiki-editpage.tpl is used to show what will be different about the tiki-editpage.tpl that is being created in /templates/styles/mozad/. Showing the new template wouldn't be helpful. Since the default tiki-editpage.tpl is used currently, this patch can be used for testing directly, however.
* preg_match is called 12417 vs 96025. That's a huge difference.
* We need to test to make sure CSAT stuff isn't affected on KB articles. It shouldn't be, but, it's something to check.
Attachment #369565 - Flags: review?(laura)
Comment on attachment 369565 [details] [diff] [review]
Get rid of poll processing in tiki-editpage.php

wfm.  Let's get it into trunk so everybody can hit it on stage.
Attachment #369565 - Flags: review?(laura) → review+
Also: it seems hella faster.
(Assignee)

Comment 12

9 years ago
This is in trunk at r23844.
(In reply to comment #12)
> This is in trunk at r23844.

I've tested both page creation and page editing, and both are expedient and I don't see any regression from this patch (on staging).
Merge please!  :)
(Assignee)

Comment 15

9 years ago
At r23854 in prod.

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Seems to be faster when it works, but we're also still affected by bug 480415, as cilias points out; not sure what the criteria are for this bug's verifcation, in light of that.
I'm ok with calling this one done since it intended to be l10n dashboard specific, and focusing on that one.  I'm hoping the database upgrade will help as well.
OK, thanks -- I agree, and every new (tigher-focused) bug should help us make specific and more measurable tweaks.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

9 years ago
Blocks: 480415

Updated

9 years ago
Whiteboard: tiki_triage

Updated

9 years ago
Whiteboard: tiki_triage → tiki_bug
Not to be upstreamed. The patch is essentially commenting out a block not used on SUMO.

Updated

9 years ago
Whiteboard: tiki_bug → tiki_bug, tiki_discuss
I've looked a bit deeper in the issue.

The code was modified since, but there are possible solutions. To begin with, there are no indexes on the table. As most searches are done on the active flag, indicating type or object, no index on that field can't be good on large volumes.

I also have the feeling the filtering is just broken as the poll name is sent as the second parameter, which is a date filter.

I would need a database dump with the usage patterns on SUMO to work with this issue.
Whiteboard: tiki_bug, tiki_discuss → tiki_bug
As expected, the bad filter clause was causing the problem. I added an index in case to avoid a table scan, but just fixing the filter changed the proportion of execution time from 23% to negligible.
Whiteboard: tiki_bug → tiki_bug, tiki_upstreamed
You need to log in before you can comment on or make changes to this bug.