Issue3

Title Poor performance when displaying issues - assignee list renders slowly
Priority bug Status resolved
Superseder Nosy List dubois, forsberg, richard, stefan
Assigned To forsberg Topics

Created on 2006-06-29.20:33:25 by forsberg, last changed 2006-12-10.13:56:21 by forsberg.

Messages
msg3 (view) Author: forsberg Date: 2006-06-29.20:33:25
Currently, the user input fields, for example assigned to, are implemented as
input fields, not as select boxes. There's a wiki page that describes how select
boxes can be generated statically using a reactor at
http://www.mechanicalcat.net/tech/roundup/wiki/UserMenu, which will give
acceptable performance even with a large set of users. 

We need to think about what's best for the python-tracker - there's a very large
amount of users, so perhaps the input-box solution is the best anyway, since a
select box would be too large to be useful? 

Another solution could be to use the same selection thingie as the nosy list -
the popup window.
msg22 (view) Author: pefu512 Date: 2006-07-07.16:48:01
In our company we started to use Roundup in October 2005.

The following is a description of what we did to customize 
Roundup according to same problem:

We introduced a special role for those members which are members
of our company and list these first in a menu given to set 
the assigned_to property.  

The Python project on sourceforge counts 68 developers as members
These might be considered to be given a similar role.  Working with
such a menu has proven to be convinient enough, if the menu is 
sorted alphabetically.  Here is some TAL-code cut from our
customized issue.item.html we use here in our Intranet-Tracker.  
It might be useful as an example:

 <td>
 <select name="assignedto">
 <option disabled="disabled">--core team--</option>
 <option value="-1" i18n:translate="">- no selection -</option>
 <tal:block tal:repeat="user python:utils.sorted_list(db.user.list())">
 <option tal:condition="python:user.hasPermission('Fixer', context._classname) a
nd utils.user_has_role(user, 'coreteam')"
         tal:attributes="
            value    user/id;
            selected python:user.id == context.assignedto"
         tal:content="python: utils.abbr_str(str(user.username)+' ('+str(user.re
alname)+')', 30)"></option>
 </tal:block>
 <option disabled="disabled">--external--</option>
 <tal:block tal:repeat="user python:utils.sorted_list(db.user.list())">
 <option tal:condition="python:user.hasPermission('Fixer', context._classname) a
nd not utils.user_has_role(user, 'coreteam')"
         tal:attributes="
            value    user/id;
            selected python:user.id == context.assignedto"
         tal:content="python: utils.abbr_str(str(user.username)+' ('+str(user.re
alname)+')', 30)"></option>
 </tal:block>
 </select>
 </td>

We have added two small Python functions to little Utility module, 
which we placed in the extensions directory of the tracker instance:
def abbr_str(string, max_length=30):
    if len(string) > max_length:
        return string[:max_length-3]+'...'
    else:
        return string

def user_has_role(user, role):
    roles = [x.lower().strip() for x in str(user.roles).split(',')]
    return role in roles

def user_compare_byid(u1, u2):
    return cmp(int(u1.id), int(u2.id))

def user_compare_byusername(u1, u2):
    return cmp(str(u1.username).lower(), str(u2.username).lower())

def sorted_list(l, cmpfunc=user_compare_byusername):
    lc = l[:]
    lc.sort(cmpfunc)
    return lc

def init(instance):
    # all functions registered here will be directly callable
    # from template code using expressions like 'python: utils.funcname()'
    instance.registerUtil('abbr_str', abbr_str)
    instance.registerUtil('user_has_role', user_has_role)
    instance.registerUtil('sorted_list', sorted_list)
    instance.registerUtil('user_compare_byid', user_compare_byid)

All this has been done with Roundup 0.8.2 as it came with Debian stable.
May with Roundup 1.x more elegant solutions are possible.

Best Regards,
Peter Funk
msg210 (view) Author: forsberg Date: 2006-12-09.21:24:06
We need to do something about this - performance is horrible, even with the
current code in issue.item.html that tries to limit the list to only developers.
The problem is that roundup runs one select query per user to fetch information
on the user. With many users, this takes time. 

Loading up an issue when logged in as devel currently takes about 20 seconds
which is not acceptable.

If any of you would like to dig into this issue, I'd be very happy.
msg211 (view) Author: stefan Date: 2006-12-09.21:28:01
Adding Richard in case he has ideas.

We ought to compress the all queries into a single
SELECT call. I'm not sure whether that is currently
possible. But it should. :-)
msg214 (view) Author: forsberg Date: 2006-12-09.23:48:40
Hmm.. if the roles property on the user objects were a Multilink to a "roles"
class, and we searched for roles instead of permissons, this would of course be
very easy, but totally against the security system as it looks today, and
there's probably a whole bunch of good reasons the security system looks like it
does today :-).
msg217 (view) Author: richard Date: 2006-12-09.23:53:41
There's nothing stopping you from adding a checkbox on users for filtering for 
assignedto. You don't *have* to use the permission system to do the filtering.
msg218 (view) Author: richard Date: 2006-12-09.23:55:32
Oh, and the latest version of Roundup has Class.filter_sql() if that helps at 
all :)
msg219 (view) Author: forsberg Date: 2006-12-10.00:45:53
Richard Jones <metatracker@psf.upfronthosting.co.za> writes:

> Oh, and the latest version of Roundup has Class.filter_sql() if that helps at > all :)

Ah. 

I'll apply the following patch:

-- snip --
Index: schema.py
===================================================================
--- schema.py   (revision 52977)
+++ schema.py   (working copy)
@@ -168,11 +168,7 @@
     db.security.addPermissionToRole('Developer', 'Edit', cl)
     db.security.addPermissionToRole('Developer', 'Create', cl)

-p = db.security.addPermission(name='Debugger', klass='issue',
-                              description='User can be assigned issues')
-db.security.addPermissionToRole('Developer', p)

-
 ##########################
 # Coordinator permissions
 ##########################
Index: html/issue.item.html
===================================================================
--- html/issue.item.html        (revision 52977)
+++ html/issue.item.html        (working copy)
@@ -117,10 +117,10 @@
  <td tal:condition="context/status/is_edit_ok">
   <select name="assignee">
    <option value="-1">nobody</option>
-   <tal:block tal:repeat="user db/user/list">
-    <option tal:condition="python:user.hasPermission('Debugger', context._classname)"
-            tal:attributes="value user/id; selected python:user.id == context.assignee"
-            tal:content="user/username"></option>
+   <tal:block tal:repeat="userdata python:db._db.user.filter_sql('select id,_username from _user where _roles like \'%Developer%\'')">
+    <option tal:attributes="value python:userdata[0]; 
+                            selected python:str(userdata[0]) == context.assignee._value"
+            tal:content="python:userdata[1]"></option>
    </tal:block>
   </select>
  </td>

-- snap --

It does remove the abstraction of "Debugger" being a permission and
"Developer" being a role, but hopefully, we can live with that.

The db._db.user construction feels a bit hackish. Feel free to come up
with a better solution.

\EF
-- 
Erik Forsberg                 http://efod.se
GPG/PGP Key: 1024D/0BAC89D9
msg220 (view) Author: richard Date: 2006-12-10.01:02:45
On Sunday 10 December 2006 11:45, Erik Forsberg wrote:
> It does remove the abstraction of "Debugger" being a permission and
> "Developer" being a role, but hopefully, we can live with that.

I've not been involved in that discussion but it seems overkill :)

> The db._db.user construction feels a bit hackish. Feel free to come up
> with a better solution.

Looks appropriate to me.
msg223 (view) Author: stefan Date: 2006-12-10.01:38:55
Richard Jones wrote:
> Richard Jones <richard@commonground.com.au> added the comment:
> 
> On Sunday 10 December 2006 11:45, Erik Forsberg wrote:
>> It does remove the abstraction of "Debugger" being a permission and
>> "Developer" being a role, but hopefully, we can live with that.
> 
> I've not been involved in that discussion but it seems overkill :)

What are you referring to as overkill ? The 'Developer' role or the
'Debugger' permission ? The latter is just a suggestion from the
roundup docs cast into our own context, and the former is used in
other places, too, i.e. where we want to restrict write-access to
certain properties, so it seems to be just the right tool for the job.

Regards,
		Stefan

-- 

      ...ich hab' noch einen Koffer in Berlin...
msg225 (view) Author: richard Date: 2006-12-10.01:44:02
On Sunday 10 December 2006 12:38, Stefan Seefeld wrote:
> > Richard Jones <richard@commonground.com.au> added the comment:
> >
> > On Sunday 10 December 2006 11:45, Erik Forsberg wrote:
> >> It does remove the abstraction of "Debugger" being a permission and
> >> "Developer" being a role, but hopefully, we can live with that.
> >
> > I've not been involved in that discussion but it seems overkill :)
>
> What are you referring to as overkill ? The 'Developer' role or the
> 'Debugger' permission ? The latter is just a suggestion from the
> roundup docs cast into our own context, and the former is used in
> other places, too, i.e. where we want to restrict write-access to
> certain properties, so it seems to be just the right tool for the job.

Sorry, I didn't intend to confuse with that silly statment. I'm still reeling 
a little from the aftermath of OSDC ;)

My poorly-made point was that you're most likely only going to have Developers 
being Debuggers, so you can equate the two and optimise for that in the 
template. Using Debugger/Developer in other places to perform permission 
checks is a good thing.

Sorry if this comes across a little rambling :(
msg226 (view) Author: stefan Date: 2006-12-10.01:50:35
Richard Jones wrote:

> Sorry, I didn't intend to confuse with that silly statment. I'm still reeling 
> a little from the aftermath of OSDC ;)
> 
> My poorly-made point was that you're most likely only going to have Developers 
> being Debuggers, so you can equate the two and optimise for that in the 
> template. Using Debugger/Developer in other places to perform permission 
> checks is a good thing.
> 
> Sorry if this comes across a little rambling :(

No worries, no offence taken at all. I just wanted to make sure I understand your
point.
You are totally right that the 'Debugger' permission is tied 1-1 to the 'Developer'
role, and its only use is to generate that reduced menu. All the better if it isn't
needed. I wasn't aware the menu could be generated without it.

Thanks,
		Stefan

-- 

      ...ich hab' noch einen Koffer in Berlin...
msg231 (view) Author: forsberg Date: 2006-12-10.11:35:49
On the same theme (performance), I changed the "Depends on" search
interface from a select box to a simple text box. That brought down
the time needed to render the search interface from somewhere around 2
minutes to 693 ms.

\EF
-- 
Erik Forsberg                 http://efod.se
GPG/PGP Key: 1024D/0BAC89D9
History
Date User Action Args
2006-12-10 13:56:21forsbergsetassignedto: forsberg
2006-12-10 11:43:41forsbergsetstatus: chatting -> resolved
2006-12-10 11:35:49forsbergsetmessages: + msg231
2006-12-10 01:50:35stefansetmessages: + msg226
2006-12-10 01:44:02richardsetmessages: + msg225
2006-12-10 01:38:55stefansetstatus: resolved -> chatting
messages: + msg223
2006-12-10 01:09:09forsbergsetstatus: chatting -> resolved
2006-12-10 01:02:45richardsetstatus: resolved -> chatting
messages: + msg220
2006-12-10 00:45:53forsbergsetstatus: chatting -> resolved
messages: + msg219
2006-12-09 23:55:32richardsetmessages: + msg218
2006-12-09 23:53:41richardsetmessages: + msg217
2006-12-09 23:48:40forsbergsetmessages: + msg214
2006-12-09 21:28:01stefansetnosy: + richard
messages: + msg211
2006-12-09 21:24:06forsbergsetpriority: wish -> bug
nosy: + dubois, stefan
messages: + msg210
title: Implement user select list -> Poor performance when displaying issues - assignee list renders slowly
2006-07-21 10:39:24forsbergsetpriority: feature -> wish
2006-07-07 16:48:02pefu512setstatus: unread -> chatting
messages: + msg22
2006-06-29 20:33:25forsbergcreate