Meteor-Community-Packages / meteor-roles

Authorization package for Meteor, compatible with built-in accounts packages
http://meteor-community-packages.github.io/meteor-roles/
MIT License
921 stars 168 forks source link

performance of role checking #191

Closed jedwards1211 closed 8 years ago

jedwards1211 commented 8 years ago

Call this a premature concern if you want, but if I have 100s of roles representing granular permissions, I don't like the thought of the server having to do 50 string comparisons on average before every user operation. Especially since my app will be running on a Raspberry Pi.

There's a very simple optimization: this package could just just cache maps with the (Flow type) structure {[userId: string]: {[group: string]: {[role: string]: true}}} for each user, and only recompute these when a user's roles is !== the last seen value. Then the userIsInRole check, when the cache is up-to-date, would be (basically) _.every(roles, role => roleCache[_.isString(user) ? user : user._id][group][role]).

Would you be open to a PR for this?

mitar commented 8 years ago

Have you looked into v2.0 branch? I do not think there is this problem anymore.

jedwards1211 commented 8 years ago

I haven't looked into all the details of how you check for roles now, but I like the v2.0 schema a lot better.

mitar commented 8 years ago

I think I will close this. If you have concrete numbers that performance of v2.0 checking is slow, show them.

We cannot change much with v1.0 because this would break backwards compatibility.

jedwards1211 commented 8 years ago

Fine with me, thanks!