Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async support #120

Closed
awemulya opened this issue Feb 9, 2021 · 10 comments
Closed

async support #120

awemulya opened this issue Feb 9, 2021 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@awemulya
Copy link

awemulya commented Feb 9, 2021

Do you have plan to adopt async features.??

Its too slow as compared to Nodejs and Go Implementations.

@wakemaster39
Copy link

wakemaster39 commented Feb 10, 2021

There are two parts to casbin, loading the policy and enforcing a policy. For loading a policy, in theory async could be used for allowing parallel execution while a async compatible adapter loaded the data from a database. There are some people interested doing this in the sqlalchemy adapter.

The second part of enforcing the policy is purely in memory and CPU bound. I have been through this code very extensively and there are no operations that are off loadable. The issue is casbin is bottle necked by an O(n) operation where n is the number of policy lines. You are just fighting the speed of python.

I went into whats going on pretty in-depth here #71 (comment) and created a drop in extension for casbin https://pypi.org/project/fastbin/ that replaces the majority of the O(n) operations with a series of O(1) look ups to get the minimal set of lines for the O(n) operation.

As long as you can align your policy with its caching strategy, it is blazing fast. I work with ~500k policy lines and all operations outside the initial load take on the order of milliseconds.

@awemulya
Copy link
Author

I was planning to write async sqlalchemy adapter but we cant overide sync methods of base class by async methods in child class.
SInce core pycasbin is sync I dont think its possible to write async adapters.

@awemulya
Copy link
Author

@wakemaster39 loading policies from adapters can be improved If core Enforce methods were async.

@manojchapagain
Copy link

manojchapagain commented Feb 10, 2021

For loading a policy, in theory async could be used for allowing parallel execution while a async compatible adapter loaded the data from a database. There are some people interested doing this in the sqlalchemy adapter.

@wakemaster39 @awemulya Writing standalone async sqlalchemy adapter is not enough because CoreEnforcer don't call load_policy as coroutine function. My understanding could be deficient but here loading policy from adapter is not awaitable so doesn't involve the event loop.

@wakemaster39
Copy link

wakemaster39 commented Feb 10, 2021

Well yes and no, the biggest issue here is async/await are not callable inside of an __init__ method and you need to do some special @classmethod that would wrap up the initialization process for you and whatever async specialness you needed. Therefore you would need to make sure that load_policy was never called in the __init__ process.

To that end, why not just bypass some of the casbin nicety code like you need to do for filtered policies to work?

adapter = YourAsyncAdapter()
e = Enforcer("rbac_model.conf", None, True)  # adapter is set to None so nothing is loaded on initialization
e.set_adapter(adapter)
await e.adapter.load_policy()
e.build_role_links() # If you want this or care

I think this is exactly what the async @classmethod that you would want to to create would do and I think you could just document this in your async adapter and be off to the races with 0 changes.

Don't let this stop you from pursuing this though, the guys maintaining this could see it differently than I did. I went through this process of trying to speed up casbin for my usage of it and determined I want to load the policy fresh on every web call and do auth as also the first op in a call chain. I then implemented the filtered policy support instead of async/await on load because it wouldn't change the length of a request which I was concerned about and filtered policies would. I also wrote some magic with my fastbin implementation that loads models on demand as you attempt to enforce them so I don't have to explicitly call load_filtered_policy() ever and it just happens in the background.

Just some more things to think about as your pursue performance, as I agree the default casbin is slow as things get large and there likely isn't one answer.

@hsluoyz hsluoyz self-assigned this Feb 10, 2021
@hsluoyz hsluoyz added the enhancement New feature or request label Feb 10, 2021
@hsluoyz
Copy link
Member

hsluoyz commented Mar 12, 2021

@Zxilly

@Zxilly
Copy link
Contributor

Zxilly commented Mar 12, 2021

@hsluoyz There is a lot work to do. Will do this.

@DarcJC
Copy link

DarcJC commented Aug 1, 2021

This might temporarily fix the problem if someone need SyncedEnforcer ?

Code

@ffyuanda
Copy link
Member

ffyuanda commented Aug 1, 2021

@DarcJC will take a look, thanks!

@hsluoyz
Copy link
Member

hsluoyz commented Jan 24, 2023

@hsluoyz hsluoyz closed this as completed Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

7 participants