Posted by mikeytown2 on February 16, 2012 at 11:39pm
7 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | browser system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | needs backport to D6, needs backport to D7, Performance |
Issue Summary
Function calls require_once so clearing this static is pointless.
On cache flush:
Before:
Total Self: 94ms
Total Cumulative: 163ms
Calls: 2,982
After:
Total Self: 49ms
Total Cumulative: 72ms
Calls: 2,982
Total savings ~90ms.
Comments
#1
#2
Non Cache Flush:
Before:
Total Self: 21ms
Total Cumulative: 27ms
Calls: 312
After:
Total Self: 15ms
Total Cumulative: 16ms
Calls: 311
total savings ~10ms
#3
#1: drupal-1443308-1-add-static-cache-to-module_load_include.patch queued for re-testing.
#4
No testing results for a month??
#5
patch is still green in #1 so it passed all tests.
#6
So, there are plans to apply it to next release or you need more testers?
#7
This is core so I have no control over if and when this goes in. The first step would be for the status to change from needs review to reviewed & tested by the community. That requires people saying that this patch works. So far I'm the only one who has stated that.
#8
Can you explain what this patch does, I don't get it. Thx.
#9
Great question as the patch in #1 was missing an important bit of code. Also looks like on my test environment this doesn't show any sort of improvement as before... So this patch is useful if you have code stored on a slow drive like NFS. We recently switched code off of NFS so we don't see the improvements now. Oh well...
On cache flush:
module_load_include()
Before:
Total Self: 43ms
Total Cumulative: 85ms
Calls: 1,702
After:
Total Self: 45ms
Total Cumulative: 84ms
Calls: 1,995
Total savings ~1ms.
drupal_get_path()
Before:
Total Self: 40ms
Total Cumulative: 55ms
Calls: 3,015
After:
Total Self: 39ms
Total Cumulative: 54ms
Calls: 2,925
Total savings ~1ms.
is_file()
Before:
Calls: 1,712
After:
Calls: 1,612
Total savings ~100 less calls to built in PHP function is_file.
Conclusion: Unless your code is stored on a slow drive, this patch is pointless.
#10
In general, the share is growing of hosting environments where the VPS/user VMs are on relatively light and inexpensive servers and files are on SAN/NAS/dedicated *raid storage machines, so this patch still makes sense, right?
p.s. I am just curious, because found link to this patch in many discussions, where drupal is running slow because of quantity of files and similar stuff.
#11
Well, if we can prove that this improves performance on some environments without hurting it (noticably) on others, that would be a strong case to get this in. Especially if those environments are shared hosters, which Drupal generally does not perform well for.
I'm pretty sure this applies to Drupal 8 as well, and hence should go there first, but could be backported all the way to Drupal 6. Adding tags accordingly.