From 3d436d3664b122701acf8fe5a65ef333b256e014 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Fri, 11 Sep 2020 12:56:15 +0100 Subject: [PATCH] Improve linkage cache linkage_cache_performance - Use reference counting so nested `CacheStoreDatabase.use` share the same underlying database (rather than rereading it every time). - Only write out the cache database file when it has changed. - Cleanup cache database entries on formula or full `brew cleanup`. Fixes #8690 --- Library/Homebrew/cache_store.rb | 50 ++++++++++++++++++++--- Library/Homebrew/cleanup.rb | 20 +++++++-- Library/Homebrew/test/cache_store_spec.rb | 10 ++--- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index 842161ec1f..6dd3768aeb 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -13,14 +13,33 @@ class CacheStoreDatabase # @param [Symbol] type # @yield [CacheStoreDatabase] self def self.use(type) - database = CacheStoreDatabase.new(type) - return_value = yield(database) - database.close_if_open! + @db_type_reference_hash ||= {} + @db_type_reference_hash[type] ||= {} + type_ref = @db_type_reference_hash[type] + + type_ref[:count] ||= 0 + type_ref[:count] += 1 + + type_ref[:db] ||= CacheStoreDatabase.new(type) + + return_value = yield(type_ref[:db]) + if type_ref[:count].positive? + type_ref[:count] -= 1 + else + type_ref[:count] = 0 + end + + if type_ref[:count].zero? + type_ref[:db].write_if_dirty! + type_ref.delete(:db) + end + return_value end # Sets a value in the underlying database (and creates it if necessary). def set(key, value) + dirty! db[key] = value end @@ -35,12 +54,13 @@ class CacheStoreDatabase def delete(key) return unless created? + dirty! db.delete(key) end # Closes the underlying database (if it is created and open). - def close_if_open! - return unless @db + def write_if_dirty! + return unless dirty? cache_path.dirname.mkpath cache_path.atomic_write(JSON.dump(@db)) @@ -76,6 +96,13 @@ class CacheStoreDatabase db.empty? end + # Performs a `each_key` on the underlying database. + # + # @return [Array] + def each_key(&block) + db.each_key(&block) + end + private # Lazily loaded database in read/write mode. If this method is called, a @@ -98,6 +125,7 @@ class CacheStoreDatabase # @return [nil] def initialize(type) @type = type + @dirty = false end # The path where the database resides in the `HOMEBREW_CACHE` for the given @@ -107,6 +135,18 @@ class CacheStoreDatabase def cache_path HOMEBREW_CACHE/"#{@type}.json" end + + # Sets that the cache needs written to disk. + def dirty! + @dirty = true + end + + # Returns `true` if the cache needs written to disk. + # + # @return [Boolean] + def dirty? + @dirty + end end # diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb index 1528125b1e..bf0f424ae1 100644 --- a/Library/Homebrew/cleanup.rb +++ b/Library/Homebrew/cleanup.rb @@ -180,7 +180,7 @@ module Homebrew def clean!(quiet: false, periodic: false) if args.empty? Formula.installed.sort_by(&:name).each do |formula| - cleanup_formula(formula, quiet: quiet, ds_store: false) + cleanup_formula(formula, quiet: quiet, ds_store: false, cache_db: false) end cleanup_cache cleanup_logs @@ -188,7 +188,7 @@ module Homebrew prune_prefix_symlinks_and_directories unless dry_run? - cleanup_old_cache_db + cleanup_cache_db rm_ds_store HOMEBREW_CACHE.mkpath FileUtils.touch PERIODIC_CLEAN_FILE @@ -225,11 +225,12 @@ module Homebrew @unremovable_kegs ||= [] end - def cleanup_formula(formula, quiet: false, ds_store: true) + def cleanup_formula(formula, quiet: false, ds_store: true, cache_db: true) formula.eligible_kegs_for_cleanup(quiet: quiet) .each(&method(:cleanup_keg)) cleanup_cache(Pathname.glob(cache/"#{formula.name}--*")) rm_ds_store([formula.rack]) if ds_store + cleanup_cache_db(formula.rack) if cache_db cleanup_lockfiles(FormulaLock.new(formula.name).path) end @@ -387,12 +388,23 @@ module Homebrew FileUtils.rm_rf portable_rubies_to_remove end - def cleanup_old_cache_db + def cleanup_cache_db(rack = nil) FileUtils.rm_rf [ cache/"desc_cache.json", cache/"linkage.db", cache/"linkage.db.db", ] + + CacheStoreDatabase.use(:linkage) do |db| + break unless db.created? + + db.each_key do |keg| + next if rack.present? && !keg.start_with?("#{rack}/") + next if File.directory?(keg) + + LinkageCacheStore.new(keg, db).delete! + end + end end def rm_ds_store(dirs = nil) diff --git a/Library/Homebrew/test/cache_store_spec.rb b/Library/Homebrew/test/cache_store_spec.rb index 8981b741be..0365b3423b 100644 --- a/Library/Homebrew/test/cache_store_spec.rb +++ b/Library/Homebrew/test/cache_store_spec.rb @@ -9,9 +9,9 @@ describe CacheStoreDatabase do let(:type) { :test } it "creates a new `DatabaseCache` instance" do - cache_store = double("cache_store", close_if_open!: nil) + cache_store = double("cache_store", write_if_dirty!: nil) expect(described_class).to receive(:new).with(type).and_return(cache_store) - expect(cache_store).to receive(:close_if_open!) + expect(cache_store).to receive(:write_if_dirty!) described_class.use(type) { |_db| } end end @@ -92,10 +92,10 @@ describe CacheStoreDatabase do end end - describe "#close_if_open!" do + describe "#write_if_dirty!" do context "database open" do it "does not raise an error when `close` is called on the database" do - expect { subject.close_if_open! }.not_to raise_error(NoMethodError) + expect { subject.write_if_dirty! }.not_to raise_error(NoMethodError) end end @@ -105,7 +105,7 @@ describe CacheStoreDatabase do end it "does not raise an error when `close` is called on the database" do - expect { subject.close_if_open! }.not_to raise_error(NoMethodError) + expect { subject.write_if_dirty! }.not_to raise_error(NoMethodError) end end end