search

dkubb / memoizable

Memoize method return values
MIT License
111 stars 10 forks source link

upgrade thread_safe to concurrent-ruby? #19

Closed fnordfish closed 4 years ago

fnordfish commented 7 years ago

headius/thread_safe was merged into https://github.com/ruby-concurrency/concurrent-ruby and is no longer maintained.

This makes use of the new Concurrent::Map, replacing ThreadSafe::Cache

fnordfish commented 7 years ago

concurrent-ruby dropped support for ruby < 1.9.3 (and corresponding JRuby, Rubinius). This is obviously a deal breaker if memoizable wants/needs to support older rubies.

mbj commented 7 years ago

concurrent-ruby dropped support for ruby < 1.9.3 (and corresponding JRuby, Rubinius). This is obviously a deal breaker if memoizable wants/needs to support older rubies.

I'd be totally fine to support only 2.0 upwards. And I see a high chance in @dkubb agreeing to such a change.

I'm not yet 100% sure if we do want concurrent-ruby filling for thread_save I've yet to look at the implementation. As I need memoizable transitively for my mutant tool and I need to have it run on 2.4 soon I'll have to look into it.

mbj commented 4 years ago

@fnordfish concurrent-ruby is not the most logical choice anymore. its quite a heavy dependency for the "simple" operation memoizable does.

Instead I'd prefer to use my much smaller variable gem to back the Memory class with an IVar.

The main benefit is that this gem is fully mutation tested. Where concurrent-ruby is not.

fnordfish commented 4 years ago

@mbj, fair enough. Should we close this one then?

mbj commented 4 years ago

@fnordfish Lets do so.