cache_store: use JSON.

After all our recent troubles with DBM I figured I'd benchmark the
performance of DBM vs. JSON. At read time (what we care more about) the
performance is pretty much identical and JSON is only 1.5x slower at
write time. This seems worth it for the reliability increases to avoid
messing with unreliable native code.

Library/Homebrew/cache_store.rb

@@ -1,6 +1,4 @@
-require "dbm"
 require "json"
-require "timeout"
 
 #
 # `CacheStoreDatabase` acts as an interface to a persistent storage mechanism
@@ -38,9 +36,10 @@ class CacheStoreDatabase
     db.delete(key)
   end
 
-  # Closes the underlying database (if it created and open).
+  # Closes the underlying database (if it is created and open).
   def close_if_open!
-    @db&.close
+    return unless @db
+    cache_path.atomic_write(JSON.dump(@db))
   end
 
   # Returns `true` if the cache file has been created for the given `@type`
@@ -52,55 +51,18 @@ class CacheStoreDatabase
 
   private
 
-  # The mode of any created files will be 0664 (that is, readable and writable
-  # by the owner and the group, and readable by everyone else). Files created
-  # will also be modified by the process' umask value at the time of creation:
-  #   https://docs.oracle.com/cd/E17276_01/html/api_reference/C/envopen.html
-  DATABASE_MODE = 0664
-
-  # Spend 5 seconds trying to read the DBM file. If it takes longer than this it
-  # has likely hung or segfaulted.
-  DBM_TEST_READ_TIMEOUT = 5
-
   # Lazily loaded database in read/write mode. If this method is called, a
   # database file with be created in the `HOMEBREW_CACHE` with name
   # corresponding to the `@type` instance variable
   #
-  # @return [DBM] db
+  # @return [Hash] db
   def db
-    # DBM::WRCREAT: Creates the database if it does not already exist
     @db ||= begin
-      HOMEBREW_CACHE.mkpath
-      if created?
-        dbm_test_read_cmd = SystemCommand.new(
-          ENV["HOMEBREW_RUBY_PATH"],
-          args: [
-            "-rdbm",
-            "-e",
-            "DBM.open('#{dbm_file_path}', #{DATABASE_MODE}, DBM::READER).values.size",
-          ],
-          print_stderr: false,
-          must_succeed: true,
-        )
-        dbm_test_read_success = begin
-          Timeout.timeout(DBM_TEST_READ_TIMEOUT) do
-            dbm_test_read_cmd.run!
-            true
-          end
-        rescue ErrorDuringExecution, Timeout::Error
-          odebug "Failed to read #{dbm_file_path}!"
-          begin
-            Process.kill(:KILL, dbm_test_read_cmd.pid)
-          rescue Errno::ESRCH
-            # Process has already terminated.
-            nil
-          end
-          false
-        end
-        cache_path.delete unless dbm_test_read_success
-      end
-      DBM.open(dbm_file_path, DATABASE_MODE, DBM::WRCREAT)
+      JSON.parse(cache_path.read) if created?
+    rescue JSON::ParserError
+      nil
     end
+    @db ||= {}
   end
 
   # Creates a CacheStoreDatabase
@@ -111,20 +73,12 @@ class CacheStoreDatabase
     @type = type
   end
 
-  # `DBM` appends `.db` file extension to the path provided, which is why it's
-  # not included
-  #
-  # @return [String]
-  def dbm_file_path
-    "#{HOMEBREW_CACHE}/#{@type}"
-  end
-
   # The path where the database resides in the `HOMEBREW_CACHE` for the given
   # `@type`
   #
   # @return [String]
   def cache_path
-    Pathname("#{dbm_file_path}.db")
+    HOMEBREW_CACHE/"#{@type}.json"
   end
 end
 
@@ -166,21 +120,4 @@ class CacheStore
 
   # @return [CacheStoreDatabase]
   attr_reader :database
-
-  # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data,
-  # to convert the ruby string back into a ruby `Hash`, the string is converted
-  # into a JSON compatible string in `ruby_hash_to_json_string`, where it may
-  # later be parsed by `JSON.parse` in the `json_string_to_ruby_hash` method
-  #
-  # @param  [Hash] ruby `Hash` to be converted to `JSON` string
-  # @return [String]
-  def ruby_hash_to_json_string(hash)
-    hash.to_json
-  end
-
-  # @param  [String] `JSON` string to be converted to ruby `Hash`
-  # @return [Hash]
-  def json_string_to_ruby_hash(string)
-    JSON.parse(string)
-  end
 end

Library/Homebrew/linkage_cache_store.rb

@@ -35,7 +35,7 @@ class LinkageCacheStore < CacheStore
       EOS
     end
 
-    database.set @keg_path, ruby_hash_to_json_string(hash_values)
+    database.set @keg_path, hash_values
   end
 
   # @param  [Symbol] the type to fetch from the `LinkageCacheStore`
@@ -68,6 +68,6 @@ class LinkageCacheStore < CacheStore
     keg_cache = database.get(@keg_path)
     return {} unless keg_cache
 
-    json_string_to_ruby_hash(keg_cache)[type.to_s]
+    keg_cache[type.to_s]
   end
 end

Library/Homebrew/test/cache_store_spec.rb

@@ -97,10 +97,6 @@ describe CacheStoreDatabase do
 
   describe "#close_if_open!" do
     context "database open" do
-      before do
-        subject.instance_variable_set(:@db, instance_double(DBM, close: nil))
-      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)
       end
@@ -118,7 +114,7 @@ describe CacheStoreDatabase do
   end
 
   describe "#created?" do
-    let(:cache_path) { Pathname("path/to/homebrew/cache/sample.db") }
+    let(:cache_path) { Pathname("path/to/homebrew/cache/sample.json") }
 
     before do
       allow(subject).to receive(:cache_path).and_return(cache_path)