{"issue":{"state":"open","created_at":"2011-11-29T07:07:45Z","updated_at":"2011-11-29T22:44:24Z","comments":1,"user":{"gravatar_id":"c257c0d93e4e2f23e71f624083dc7978","id":438552,"url":"https://api.github.com/users/jc0n","login":"jc0n","avatar_url":"https://secure.gravatar.com/avatar/c257c0d93e4e2f23e71f624083dc7978?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png"},"body":"I'm still coming up to speed on how the authorization system works but I had some thoughts:\r\n\r\nFrom what I can tell it seems that every authorization check (which there are of course many) runs with O(n) complexity where n is the number of ACL method checks required for any of the crud operations. This also does not include what appears to be a lot of time spent filtering ACLs to those actually required for the check. \r\n\r\nI admit that I have not profiled this, but I think we could dramatically improve the performance by building 4 maps for each of c, r, u, and d operations which would map from type objects to a set of ACL objects.\r\n\r\nFor selecting only relevant ACLs for those which are attribute specific, we can create a map from attribute names to a set of ACL objects. For r and u operations we can then set intersect  x and y where x is the set of ACL objects given by looking up ACLs by object type and y is the set union of ACLs given by looking up each requested attribute name. I would expect the extra memory overhead to be minimal since we're only storing references to existing objects. \r\n\r\nFurther gains come from this computation being done once (ie building the dictionaries). A lot of the work filtering ACLs is currently done in every check `Authorizer._get_relevant_acls_for_attributes` and appears to be O(nm) where n is the number of ACLs and m is the number of requested fields.\r\n\r\nIt also looks like we **could** support using the class object as the actee for create operations. Which would provide an easy fix for authorizing #40.","assignee":null,"id":2379508,"pull_request":{"patch_url":null,"html_url":null,"diff_url":null},"labels":[{"name":"performance","color":"FF6600","url":"https://api.github.com/repos/TireSwingSoftware/openassign-server/labels/performance"}],"number":50,"title":"Authorizer performance improvements","html_url":"https://github.com/TireSwingSoftware/openassign-server/issues/50","closed_at":null,"url":"https://api.github.com/repos/TireSwingSoftware/openassign-server/issues/50","milestone":null},"action":"created","comment":{"created_at":"2011-11-29T22:44:24Z","updated_at":"2011-11-29T22:44:24Z","user":{"gravatar_id":"06b29370cfbe838c3ed487118738d24e","id":53091,"url":"https://api.github.com/users/mhrivnak","login":"mhrivnak","avatar_url":"https://secure.gravatar.com/avatar/06b29370cfbe838c3ed487118738d24e?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png"},"body":"Interesting idea, and I'm sure it would improve the performance of how we identify which roles will satisfy the currently requested permission.\r\n\r\nPerhaps a less direct approach that would probably be simpler overall and easier to implement would be to cache the results of Authorizer._get_relevant_acls_for_attributes().  We could create a hash based on a combination of the arguments passed in and use that as a key to cache the results.\r\n\r\nHowever, I think you'll find that since the total number of ACLs is really quite small and is never likely to even top 100, this operation is very fast already and represents a tiny fraction of the total time it takes to handle a request.\r\n\r\nMy biggest concern about authorizer performance is the check methods, which require a substantial amount of database activity to verify relationships.  Work has been done to minimize that, such as using django's \"QuerySet.select_related()\" method, and caching the results of certain methods.  My recent work to cache AuthToken objects with associated user data also helped.\r\n\r\nIn any case, I think this is a good idea that has to be a very low priority right now.  We need to finish implementing features ASAP, and then we can profile to identify performance problems.  Once identified and measured, we can address those performance issues.","id":2951805,"url":"https://api.github.com/repos/TireSwingSoftware/openassign-server/issues/comments/2951805"}}