Possible big optimisation of $auth->acl-get(s)

General discussion of development ideas and the approaches taken in the 3.x branch of phpBB. The current feature release of phpBB 3 is 3.3/Proteus.
Forum rules
Please do not post support questions regarding installing, updating, or upgrading phpBB 3.3.x. If you need support for phpBB 3.3.x please visit the 3.3.x Support Forum on phpbb.com.

If you have questions regarding writing extensions please post in Extension Writers Discussion to receive proper guidance from our staff and community.
User avatar
Geolim4
Registered User
Posts: 10
Joined: Thu Sep 13, 2012 11:14 pm

Possible big optimisation of $auth->acl-get(s)

Post by Geolim4 »

Hello
i've see a thing with the phpbb FR Team: For all Mod we made, we call many time $auth->acl_get('AN_ACL') but we've see if we assign to a var like: $acp_acl = $auth->acl_get('a_') the spend time php is very reduced
50 call with $auth->acl_get('a_') 0.065s
1 call of $auth->acl_get('a_') with 49 call of $acp_acl 0.0.42 s
i think you should working on it with the phpbb dev team...
And add for exemple a rules in the guideline to use the following vars as an acl:

Code: Select all

$acl_a_,  $acl_m_; //etc...    
It can be increase Mod quality and reduce CPU loading...
Best regards.
User avatar
brunoais
Registered User
Posts: 964
Joined: Fri Dec 18, 2009 3:55 pm

Re: Possible big optimisation of $auth->acl-get(s)

Post by brunoais »

That looks like weird...
Do some more testing and pls get some code (and mark the time it took to work) to try to understand what's happening for that to happen. Reading a variable is supposed to be faster than executing a method.
Senky
Extension Customisations
Extension Customisations
Posts: 315
Joined: Thu Jul 16, 2009 4:41 pm

Re: Possible big optimisation of $auth->acl-get(s)

Post by Senky »

I do not see any problem assigning result of acl_get in variable and use it afterwards. phpBB simply provides API to get acl options. Or do you mean that it should be remade in core phpBB files so that they would use variables instead of methods?

I think better option would be to add single-script-call cache. So result of acl_get with specified parameter would be fetched from DB, and saved in cache (in this case, variable used as cache should be enough), then, when you call acl_get with the same input value again, it will return you value of that variable, so that it does not do another call to DB.

Of course, you need to take into consideration MOD/extension can change permissions as you run, so method acl_set (or whatever it is called) should clear cache of acl_get.

I hope you understand and sorry for long reading if this is already implemented.
User avatar
brunoais
Registered User
Posts: 964
Joined: Fri Dec 18, 2009 3:55 pm

Re: Possible big optimisation of $auth->acl-get(s)

Post by brunoais »

BTW, geolim4. In which version did you test that?
If you didn't test on 3.1dev then you should. There are a lot of code changes in there.
User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: Possible big optimisation of $auth->acl-get(s)

Post by bantu »

Please show example code.
User avatar
Geolim4
Registered User
Posts: 10
Joined: Thu Sep 13, 2012 11:14 pm

Re: Possible big optimisation of $auth->acl-get(s)

Post by Geolim4 »

http://pastebin.com/VNiQKqPK
All is there...
Result returned:

Code: Select all

While acl(method) time:0.00136113166809s
While acl(var) time:8.10623168945E-5s
No need to explain i think... :roll: :roll:

Look @ search.php, viewtopic.php, and function_display.php...
Search for $auth->acl_get(s) method in while and foreach
User avatar
brunoais
Registered User
Posts: 964
Joined: Fri Dec 18, 2009 3:55 pm

Re: Possible big optimisation of $auth->acl-get(s)

Post by brunoais »

Yep, we could remove some repetitions and save stuff to the variable.
That is a quite significant improvement, yeah...

Care to make the PR?
User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: Possible big optimisation of $auth->acl-get(s)

Post by bantu »

The example provided is not a real life example. Of course acl_get() should not be called in loops. Feel free to send patches against phpBB for where this is the case.
User avatar
Geolim4
Registered User
Posts: 10
Joined: Thu Sep 13, 2012 11:14 pm

Re: Possible big optimisation of $auth->acl-get(s)

Post by Geolim4 »

Memberlist.php:
Around line 171~~

Code: Select all

		while ($row = $db->sql_fetchrow($result))

In the while you call:

Code: Select all

if ($auth->acl_get('f_list', $forum_id))
(Okey for this, it's not easy to replace by a var i agree)
but after:

Code: Select all

if ($row['group_type'] == GROUP_HIDDEN && !$auth->acl_gets('a_group', 'a_groupadd', 'a_groupdel') && $row['ug_user_id'] != $user->data['user_id'])
and again:

Code: Select all

'U_PM'				=> ($config['allow_privmsg'] && $auth->acl_get('u_sendpm') && ($row['user_allow_pm'] || $auth->acl_gets('a_', 'm_') || $auth->acl_getf_global('m_'))) ? append_sid("{$phpbb_root_path}ucp.$phpEx", 'i=pm&mode=compose&u=' . $row['user_id']) : '',
Search.php:

Code: Select all

		foreach ($rowset as $row)
Count any acl methods... try to replace some of them by a var previously declared... You'll feel the difference in big board...
But i mean for acl method, but all my previous post are available for $user class also...
The mod used method is data: $user->data['a_random_data']
Why call the method each time in all loop (foreach while eand other...)

Code: Select all

$user_data => $user->data
In most of time where you don't modify the data of current user, why call always this method??
When we can cache these data in one var instead call a method, why don't do it???
User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: Possible big optimisation of $auth->acl-get(s)

Post by nickvergessen »

Well if it is dont in a loop, the number of circles is mostly <100 and I'd say it's not really worth the confussion then...
Member of the Development-TeamNo Support via PM
Post Reply