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

Create thread-safe @cached_property #81

Closed
pared opened this issue Nov 28, 2019 · 3 comments
Closed

Create thread-safe @cached_property #81

pared opened this issue Nov 28, 2019 · 3 comments

Comments

@pared
Copy link

pared commented Nov 28, 2019

Currently, it is possible that cached_property will create multiple objects when used in multi-threaded context.

For example:

from concurrent.futures import ThreadPoolExecutor

from funcy import cached_property


class Client(object):
    pass


class Connector(object):

    @cached_property
    def client(self):

        from time import sleep
        sleep(0.1)
        return Client()

    def connect(self, arg):
        print("arg '{}', client '{}'".format( str(arg), str(self.client)))
        return 0


c = Connector()
args = [1,2,3,4,5]

with ThreadPoolExecutor(max_workers=5) as executor:
    executor.map(c.connect, args)

Will most likely create new client properties for each thread, however, if we comment out sleep
there is a chance that it will be a single object.

It would be nice to make @cached_property thread-safe or create a new decorator that could do that.

@Suor
Copy link
Owner

Suor commented Nov 28, 2019

Good idea, @pared. I thought about this myself too ;)

@Suor Suor closed this as completed in f42b8a6 Nov 28, 2019
@DavidMStraub
Copy link

Apologies for asking a question on a resolved issue, but I didn't want to open a new one.

Question: given that a cached_property decorator has been added to the standard library (functools) in Python 3.8, I was wondering whether you are planning to use that as default (if it exists) or whether your own implementation will be kept (perhaps it is more powerful).

Thanks for this great library.

@Suor
Copy link
Owner

Suor commented Jan 2, 2020

You should really create new issues for things like this, it might be simply lost here)

Python 3.8 @cached_property is not entirely the same. The main difference is that it is always thread-safe, which might make it a little bit slower on cache miss. Currently with funcy one can make it thread safe with:

@wrap_prop(threading.RLock())
@cached_property
def prop_name(self):
    # ...
    return value

Which will make us use 2 rlocks if Python 3.8 @cached_property is used. So basically funcys @cached_property is a smaller building block.

If I decide to move to Python 3.8 way I will need to use its implementation for older pythons too, otherwise it will be incosistent regarding thread-safety. Then to follow that logic I will need to make thread-safe other caching parts of funcy: @memoize, @cache, @cached_readonly, @make_lookuper, @silent_lookuper and LazyObject. This will mean I depart from current granularity approach of requiring a user to wrap things with @wrap_with and @wrap_prop manually.

So, it looks like a shift, which needs some thinking. I'll take my time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants