Project

General

Profile

Bug #7053

Unnecessary load on pages with quickform due to docs-total-space check

Added by Francis Gonzales over 6 years ago. Updated over 6 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
Global / Others / Misc
Target version:
Start date:
01/04/2014
Due date:
% Done:

30%

Estimated time:
2.00 h
Spent time:
Complexity:
Challenging
SCRUM pts - complexity:
?

Description

The method Document::documents_total_space() is loaded on each course homepage (actually, each time a main/inc/lib/pear/HTML/QuickForm/HTML_QuickForm::__construct() is called) because of an invisible quickform.

This is super inneficient, considering that University of Genève recently reported it is the largest resource consumer of all functions (a real perf killer, following Patrick's words).

If it is really necessary to have this calculation there, then it should really be loaded in AJAX, but in any case it would be better to simply get rid of it in the quickform builder (the extra visual info is not worth loading it on each course homepage).

(registered by Yannick)

History

#1

Updated by Francis Gonzales over 6 years ago

  • Description updated (diff)
#2

Updated by Yannick Warnier over 6 years ago

  • % Done changed from 0 to 20

I'm not 100% sure yet, but at first view it looks like it reduced the load average of 25% on campus.chamilo.org, passing from 8 to 6 (maybe it's due to a restart of Apache, but still... it looks suspiciously more efficient).

The api_get_course_int_id() that is just before that code block can also go away, considering it is only used for the block itself.

#3

Updated by Yannick Warnier over 6 years ago

  • Assignee changed from Julio Montoya to Francis Gonzales
#4

Updated by Yannick Warnier over 6 years ago

To be tested with xhprof first to confirm the performance gain (at least on the course homepage, but it is very likely to affect a large number of other pages).

#5

Updated by Francis Gonzales over 6 years ago

  • Assignee changed from Francis Gonzales to Yannick Warnier
  • % Done changed from 20 to 40

This change was tested in the course index page..

Run #533ae3c51f00a = using the function DocumentManager::documents_total_space.
Run #533ae40b9a10d = commenting the function DocumentManager::documents_total_space.

Overall Diff Summary
Run #533ae3c51f00a Run #533ae40b9a10d Diff Diff%
Number of Function Calls 15,457 13,899 -1,558 -10.1%
Incl. Wall Time (microsec) 217,232 104,165 -113,067 -52.0%
Incl. CPU (microsecs) 208,000 100,000 -108,000 -51.9%
Incl. MemUse (bytes) 22,724,216 6,649,464 -16,074,752 -70.7%
Incl. PeakMemUse (bytes) 22,864,432 6,735,704 -16,128,728 -70.5%
#6

Updated by Yannick Warnier over 6 years ago

  • Subject changed from Unnecessary load to Unnecessary load on pages with quickform due to docs-total-space check
#7

Updated by Yannick Warnier over 6 years ago

  • Status changed from Assigned to New
  • Assignee deleted (Yannick Warnier)
  • Target version changed from 1.9.8 to 2.0
  • % Done changed from 40 to 30

As reported by Julio on Github, this goes against a feature developed a year ago or so to fix the limit on the upload form.

Although I understand the importance of this (other) feature in terms of user interface, it doesn't justify the load that we are adding to the system (around 10%) just to make it slightly easier for some users.

Proposed solution: only calculate the total folder occupation if there is a caching mechanism involved, so we can scan the folder for real space used only from time to time (with a ttl=0 and a process when uploading a document, that updates the value kept in cache, each time we upload a document).

The global caching system will only be available in 1.10. This is why I'm moving this task to 1.10 and reducing to 30%, but technically the code to do the size counting has been removed.

As a note, we couldn't find the part where this space calculation actually gets into a MAX_FILLED_SPACE element. I understand this happens in the setMaxFileSize() function of the QuickForm.php script, but we couldn't see that it was set when loading the course homepage...

Also available in: Atom PDF