Improve performance of brew cask list

[hanxue]

See Issue #911 for original bug. This hack will list the contents of Caskroom. This should work reasonably well because the directory name of the installed cask is exactly the same as the cask name.

TODO: Use Cask::Locations::caskroom instead of absolute path name

I don't know how to use Cask::Locations::caskroom, can someone patch instead?

The previous commit has

def self.run(*arguments)
      puts_columns Cask::CLI.nice_listing(Cask.installed)
end

*arguments does not seem to be used. Can it be removed, so that there is no need to wrap the code in if-else?

[nanoxd]

@hanxue Please add tests 😄

[hanxue]

@nanoxd Sorry, I am a newbie here. Is there an example I can start from?

How can I invoke Cask::Locations.caskroom from within list.rb's self.run method?

[nanoxd]

The current tests are here and they use the following to run the list command:

lambda { Cask::CLI::List.run }

[nanoxd]

You can load the code into a REPL like IRB or Pry

[paulp]

% It's hard to believe it was once even slower - has this regressed?

% time brew cask list > /dev/null
3.497 real, 3.356 user, 0.140 sys
% time ls -1 /opt/homebrew-cask/Caskroom > /dev/null
0.003 real, 0.001 user, 0.002 sys

[rolandwalker]

@paulp it's possible that the recent Tap migration changes made it worse.

Probably the fundamental culprit is trying to match Homebrew semantics exactly. That might not even be necessary.

I never saw this issue the first time around. I'm re-opening it as a reminder to look into this.

[paulp]

I'm sure that corner cases abound somewhere, but for what it is worth, on my machine the output of the 3ms ls -1 is identical to that of the 3497ms brew cask list.

[paulp]

Or for columns, unix is still there for us.

% ls -1 /opt/homebrew-cask/Caskroom |column
adapter          caffeine                   gitx-rowanj    keyboard-cleaner    subtitles
alfred           dropbox                    google-chrome  lastpass-universal  textexpander
appcleaner       fantastical                growlnotify    launchcontrol       tvshows
asepsis          filebot                    hazel          musicbrainz-picard  vlc
bartender        firefox                    iterm2         skim                xquartz
bettertouchtool  font-sauce-code-powerline  java           sonos
bonjour-browser  font-source-code-pro       java7          sublime-text-dev

[rolandwalker]

ls is not quite the same output: brew cask list annotates problematic Casks with (!).

The perf issue arises from the fact that each installed Cask definition is loaded. (Load failures are annotated with the (!).) The Ruby instantiation cost is not the problem; rather it is the DWIM flexibility of the Cask.load interface which leads to testing many possibilities before loading the right file.

For this use case, we can tell Cask.load to do less work by feeding it the fully-qualified paths to the Cask definitions. With that and other changes in #4434, I measure about 2 orders of magnitude speed improvement for brew cask list.

[paulp]

I've seen the (!)s before, but it was very unclear what I was expected to do with that knowledge. You mean the ! means there's something wrong with the formula of an installed cask? Why would this occur in the normal course of events, and how does it bear on which casks I have installed? I think when people say "list" they are looking for the list of installed casks, and not looking to perform verification on the cask infrastructure - especially not super-expensive verification, every time.

[rolandwalker]

I guess I wasn't very clear above. Instantiating a Cask isn't expensive. The Cask.load method is expensive, because it was designed for flexible/tolerant UI, not backend speed. That was easily fixed in #4434.

Thanks for bringing the problem to my attention.