From 03af4f45a1d3f9218c8a73fd7048a14ac77c32f7 Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Thu, 15 May 2014 16:38:35 -0400 Subject: [PATCH 1/5] make cask_titles hold Pathnames, not strings --- lib/cask/scopes.rb | 2 +- lib/cask/source/uri.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cask/scopes.rb b/lib/cask/scopes.rb index 5aca9263861e..71af687abe6f 100644 --- a/lib/cask/scopes.rb +++ b/lib/cask/scopes.rb @@ -9,7 +9,7 @@ def all end def all_titles - cask_titles = Dir[tapspath.join('*', '*', 'Casks', '*.rb')] + cask_titles = Dir[tapspath.join('*', '*', 'Casks', '*.rb')].map { |d| Pathname.new(d) } cask_titles.map { |c| # => "/usr/local/Library/Taps/caskroom/example-tap/Casks/example.rb" c.sub!(/\.rb$/, '') diff --git a/lib/cask/source/uri.rb b/lib/cask/source/uri.rb index 958659e46661..4e1730f12f9d 100644 --- a/lib/cask/source/uri.rb +++ b/lib/cask/source/uri.rb @@ -1,6 +1,6 @@ class Cask::Source::URI def self.me?(query) - !!(query =~ URI.regexp) + !!(query.to_s =~ URI.regexp) end attr_reader :uri From 598f53ba48511097e8096001c486587db25eab2e Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Thu, 15 May 2014 16:49:43 -0400 Subject: [PATCH 2/5] memoize all_tapped_cask_dirs --- lib/cask.rb | 1 + lib/cask/scopes.rb | 20 +++++++++++++++++++- lib/cask/source/untapped_qualified.rb | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/cask.rb b/lib/cask.rb index 1ba8709f947e..7827badbd980 100644 --- a/lib/cask.rb +++ b/lib/cask.rb @@ -111,6 +111,7 @@ def self.set_up_taps ohai 'Adding caskroom Tap' Homebrew.install_tap(new_user, repo) end + Cask.reset_all_tapped_cask_dirs end def self.load(query) diff --git a/lib/cask/scopes.rb b/lib/cask/scopes.rb index 71af687abe6f..bba2faf26b99 100644 --- a/lib/cask/scopes.rb +++ b/lib/cask/scopes.rb @@ -8,8 +8,26 @@ def all all_titles.map { |c| self.load c } end + def all_tapped_cask_dirs + return @all_tapped_cask_dirs unless @all_tapped_cask_dirs.nil? + fq_default_tap = tapspath.join(default_tap, 'Casks') + @all_tapped_cask_dirs = Dir.glob(tapspath.join('*', '*', 'Casks')).map { |d| Pathname.new(d) } + # optimization: place the default Tap first + if @all_tapped_cask_dirs.include? fq_default_tap + @all_tapped_cask_dirs = @all_tapped_cask_dirs - [ fq_default_tap ] + @all_tapped_cask_dirs.unshift fq_default_tap + end + @all_tapped_cask_dirs + end + + def reset_all_tapped_cask_dirs + # The memoized value should be reset when a Tap is added/removed + # (which is a rare event in our codebase). + @all_tapped_cask_dirs = nil + end + def all_titles - cask_titles = Dir[tapspath.join('*', '*', 'Casks', '*.rb')].map { |d| Pathname.new(d) } + cask_titles = all_tapped_cask_dirs.map { |d| Dir.glob d.join('*.rb') }.flatten cask_titles.map { |c| # => "/usr/local/Library/Taps/caskroom/example-tap/Casks/example.rb" c.sub!(/\.rb$/, '') diff --git a/lib/cask/source/untapped_qualified.rb b/lib/cask/source/untapped_qualified.rb index 3c7f15c17a64..e84ad0ee1cfd 100644 --- a/lib/cask/source/untapped_qualified.rb +++ b/lib/cask/source/untapped_qualified.rb @@ -9,6 +9,7 @@ def self.path_for_query(query) unless Cask.tapspath.join(tap).exist? ohai "Adding new tap '#{tap}'" Homebrew.install_tap(user, repo) + Cask.reset_all_tapped_cask_dirs end Cask.tapspath.join(tap, 'Casks', "#{cask}.rb") end From c3e514defe72366a38864ad0a9c7060f41f28b83 Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Thu, 15 May 2014 16:50:42 -0400 Subject: [PATCH 3/5] optimize Cask.installed by guessing full paths --- lib/cask/scopes.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/cask/scopes.rb b/lib/cask/scopes.rb index bba2faf26b99..8f0b1284bf7c 100644 --- a/lib/cask/scopes.rb +++ b/lib/cask/scopes.rb @@ -42,8 +42,17 @@ def all_titles def installed installed_cask_dirs = Pathname.glob(caskroom.join("*")) - installed_cask_dirs.map do |dir| - Cask.load(dir.basename.to_s) + # Cask.load has some DWIM which is slow. Optimize here + # by spoon-feeding Cask.load fully-qualified paths. + # todo: speed up Cask::Source::Tapped (main perf drag is calling Cask.all_titles repeatedly) + # todo: ability to specify expected source when calling Cask.load (minor perf benefit) + installed_cask_dirs.map do |install_dir| + cask_name = install_dir.basename.to_s + path_to_cask = all_tapped_cask_dirs.find do |tap_dir| + tap_dir.join("#{cask_name}.rb").exist? + end + cask_name = path_to_cask.join("#{cask_name}.rb") if path_to_cask + Cask.load(cask_name) end end end From 3c40e82c18a7abe6afc6076670c1e8965141a59d Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Thu, 15 May 2014 16:52:17 -0400 Subject: [PATCH 4/5] generate installed list only once during list cmd --- lib/cask/cli/list.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cask/cli/list.rb b/lib/cask/cli/list.rb index a59cb99f7ca4..29b98ba47a7a 100644 --- a/lib/cask/cli/list.rb +++ b/lib/cask/cli/list.rb @@ -45,9 +45,10 @@ def self.list_files(cask) end def self.list_installed - columns = Cask.installed.map(&:to_s) + installed_casks = Cask.installed + columns = installed_casks.map(&:to_s) puts_columns columns - columns.empty? ? nil : Cask.installed.length == columns.length + columns.empty? ? nil : installed_casks.length == columns.length end def self.help From 5b2e9d1466134e300b6b1b12c5e1ceece6ca28f2 Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Thu, 15 May 2014 16:52:41 -0400 Subject: [PATCH 5/5] doc/comment: known performance issues --- lib/cask/source/tapped.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/cask/source/tapped.rb b/lib/cask/source/tapped.rb index a36e51f6afe4..e26472022ef9 100644 --- a/lib/cask/source/tapped.rb +++ b/lib/cask/source/tapped.rb @@ -4,6 +4,10 @@ def self.me?(query) end def self.path_for_query(query) + # Repeating Cask.all_titles is very slow for operations such as + # brew cask list, but memoizing the value might cause breakage + # elsewhere, given that installation and tap status is permitted + # to change during the course of an invocation. cask_with_tap = Cask.all_titles.find { |t| t.split('/').last == query.sub(/\.rb$/i,'') } if cask_with_tap user, repo, cask = cask_with_tap.split('/')