@kvokka wdyt on separating mutex logic, checking for stale cache and preparing methods list? I hope it'll be easier to read and understand.

def all_available_methods_cached
  @mutext.sync do
    @all_available_methods_cached = all_available_methods if stale_methods_cache?
    @all_available_methods_cached
  end
end

def stale_methods_cache?
  # check that modules count changed
end

def all_available_methods
  # Fetch from ObjectSpace
end

And if you already have class << self section on the top, it would be better to move this methods there.

Mutext can be also set once with a constant: MUTEXT = Mutex.new

@printercu There is no problem to refactor it, but as I see, this code was written more than 3 month ago. Now, unfortunately, I see, that this chunk is still unuseful for repo owner. If it helps somebody- awesome!

You may take this snippet and make own PW with refactored code, if you wish.

About singleton in the bottom, you, probably, missed, that it is in private section. I prefer put all private methods in 1 section.

@kvokka Ok, I'll take a look. BTW, private modifier doesn't effect class-methods: http://rubocop.readthedocs.io/en/latest/cops_lint/#lintineffectiveaccessmodifier

Or one-liner class A; private; def self.must_be_private; end; end; A.public_methods(false)

@printercu Thank you! This is good sample.

@jazzonmymind I'm almost ready with similar to this PR refactoring, I'll make PR soon. I think method cache should be global, because it doesn't depend on pry's state.

i don't understand that argument.
an instance of Pry::InputCompleter already belongs to an instance of Pry, we just re-initialise it every time Pry accepts input. Avoiding global state is always a good thing, and the only reason it has to be global is because we throw away the instance of Pry::InputCompleter at each eval tick. we could initialise one instance of Pry::InputCompleter, store the method cache on that instance, and avoid the lock altogether. locks should be a last resort, and global state should be avoided where possible. in this case, sharing the cache between all Pry::InputCompleter's has no benefit as far as i can tell, and the cache could fill up with methods that don't relate between Pry instances. it's in fact a negative imo.

Got it. On the other hand completer instance is connected to binding, and we would need to update it on every switch. IMO This would not be good design. I sew that binding is an arg of #call

what do you mean by "on every switch" ? the binding is received in Pry::InputCompleter#call, and that wouldn't change. #initialize accepts an input object(like Readline for example) and an instance of Pry.

the only difference between global and instance-local state would be that we would store the collection and count as instance variables on an instance of Pry::InputCompleter, and avoid the mutex and class-level state entirely. nothing else would change, even how bindings are passed into Pry::InputCompleter would not change.

Yes, I see. Sorry fro my comment before I've checked the code. I've added a not in #1583 (comment)