Implement copy-on-write semantics for VarSet, speeding up Environment cloning

lib/rscons/varset.rb

@@ -1,53 +1,73 @@
 module Rscons
   # This class represents a collection of variables which can be accessed
-  # as certain types
+  # as certain types.
+  # Only nil, strings, arrays, and hashes should be stored in a VarSet.
   class VarSet
-    # The underlying hash
+    # Create a VarSet.
-    attr_reader :vars
-
-    # Create a VarSet
     # @param vars [Hash] Optional initial variables.
     def initialize(vars = {})
-      if vars.is_a?(VarSet)
+      @my_vars = {}
-        @vars = vars.clone.vars
+      @coa_vars = []
-      else
+      append(vars)
-        @vars = vars
-      end
     end
 
     # Access the value of variable as a particular type
     # @param key [String, Symbol] The variable name.
     # @return [Object] The variable's value.
     def [](key)
-      @vars[key]
+      if @my_vars.include?(key)
+        @my_vars[key]
+      else
+        @coa_vars.each do |coa_vars|
+          if coa_vars.include?(key)
+            @my_vars[key] = deep_dup(coa_vars[key])
+            return @my_vars[key]
+          end
+        end
+        nil
+      end
     end
 
     # Assign a value to a variable.
     # @param key [String, Symbol] The variable name.
     # @param val [Object] The value to set.
     def []=(key, val)
-      @vars[key] = val
+      @my_vars[key] = val
     end
 
     # Check if the VarSet contains a variable.
     # @param key [String, Symbol] The variable name.
     # @return [true, false] Whether the VarSet contains a variable.
     def include?(key)
-      @vars.include?(key)
+      if @my_vars.include?(key)
+        true
+      else
+        @coa_vars.find do |coa_vars|
+          coa_vars.include?(key)
+        end
+      end
     end
 
     # Add or overwrite a set of variables
     # @param values [VarSet, Hash] New set of variables.
     def append(values)
-      values = values.vars if values.is_a?(VarSet)
+      coa!
-      @vars.merge!(deep_dup(values))
+      if values.is_a?(VarSet)
+        values.send(:coa!)
+        @coa_vars = values.instance_variable_get(:@coa_vars) + @coa_vars
+      else
+        @my_vars = deep_dup(values)
+      end
       self
     end
 
     # Create a new VarSet object based on the first merged with other.
     # @param other [VarSet, Hash] Other variables to add or overwrite.
     def merge(other = {})
-      VarSet.new(deep_dup(@vars)).append(other)
+      coa!
+      varset = self.class.new
+      varset.instance_variable_set(:@coa_vars, @coa_vars.dup)
+      varset.append(other)
     end
     alias_method :clone, :merge
 
@@ -62,13 +82,13 @@ module Rscons
       else
         if varref =~ /^(.*)\$\{([^}]+)\}(.*)$/
           prefix, varname, suffix = $1, $2, $3
-          varval = expand_varref(@vars[varname])
+          varval = expand_varref(self[varname])
           if varval.is_a?(String)
             expand_varref("#{prefix}#{varval}#{suffix}")
           elsif varval.is_a?(Array)
             varval.map {|vv| expand_varref("#{prefix}#{vv}#{suffix}")}.flatten
           else
-            raise "I do not know how to expand a variable reference to a #{varval.class.name} (from #{varname.inspect} => #{@vars[varname].inspect})"
+            raise "I do not know how to expand a variable reference to a #{varval.class.name} (from #{varname.inspect} => #{self[varname].inspect})"
           end
         else
           varref
@@ -78,9 +98,17 @@ module Rscons
 
     private
 
-    # Create a deep copy of a Hash or Array.
+    # Move all VarSet variables into the copy-on-access list.
-    # @param obj [Hash, Array] Hash or Array to deep copy.
+    def coa!
-    # @return [Hash, Array] Deep copied value.
+      unless @my_vars.empty?
+        @coa_vars.unshift(@my_vars)
+        @my_vars = {}
+      end
+    end
+
+    # Create a deep copy of an object.
+    # @param obj [nil, String, Array, Hash] Object to deep copy.
+    # @return [nil, String, Array, Hash] Deep copied value.
     def deep_dup(obj)
       obj_class = obj.class
       if obj_class == Hash

spec/rscons/varset_spec.rb

@@ -21,9 +21,9 @@ module Rscons
     end
 
     describe "#[]" do
-      v = VarSet.new({"fuz" => "a string", "foo" => 42, "bar" => :baz,
-                      "qax" => [3, 6], "qux" => {a: :b}})
       it "allows accessing a variable with its verbatim value if type is not specified" do
+        v = VarSet.new({"fuz" => "a string", "foo" => 42, "bar" => :baz,
+                        "qax" => [3, 6], "qux" => {a: :b}})
         v["fuz"].should == "a string"
         v["foo"].should == 42
         v["bar"].should == :baz
@@ -44,11 +44,21 @@ module Rscons
     describe "#include?" do
       it "returns whether the variable is in the VarSet" do
         v = VarSet.new("CFLAGS" => [], :foo => :bar)
+
         expect(v.include?("CFLAGS")).to be_true
         expect(v.include?(:CFLAGS)).to be_false
         expect(v.include?(:foo)).to be_true
         expect(v.include?("foo")).to be_false
         expect(v.include?("bar")).to be_false
+
+        v2 = v.clone
+        v2.append("bar" => [])
+
+        expect(v2.include?("CFLAGS")).to be_true
+        expect(v2.include?(:CFLAGS)).to be_false
+        expect(v2.include?(:foo)).to be_true
+        expect(v2.include?("foo")).to be_false
+        expect(v2.include?("bar")).to be_true
       end
     end
 
@@ -56,14 +66,26 @@ module Rscons
       it "adds values from a Hash to the VarSet" do
         v = VarSet.new("LDFLAGS" => "-lgcc")
         v.append("LIBS" => "gcc", "LIBPATH" => ["mylibs"])
-        v.vars.keys.should =~ ["LDFLAGS", "LIBS", "LIBPATH"]
+        expect(v["LDFLAGS"]).to eq("-lgcc")
+        expect(v["LIBS"]).to eq("gcc")
+        expect(v["LIBPATH"]).to eq(["mylibs"])
       end
+
       it "adds values from another VarSet to the VarSet" do
         v = VarSet.new("CPPPATH" => ["mydir"])
         v2 = VarSet.new("CFLAGS" => ["-O0"], "CPPPATH" => ["different_dir"])
         v.append(v2)
-        v.vars.keys.should =~ ["CPPPATH", "CFLAGS"]
+        expect(v["CFLAGS"]).to eq(["-O0"])
-        v["CPPPATH"].should == ["different_dir"]
+        expect(v["CPPPATH"]).to eq(["different_dir"])
+      end
+
+      it "does not pick up subsequent variable changes from a given VarSet" do
+        v = VarSet.new("dirs" => ["a"])
+        v2 = VarSet.new
+        v2.append(v)
+        v["dirs"] << "b"
+        expect(v["dirs"]).to eq(["a", "b"])
+        expect(v2["dirs"]).to eq(["a"])
       end
     end
 
@@ -71,17 +93,30 @@ module Rscons
       it "returns a new VarSet merged with the given Hash" do
         v = VarSet.new("foo" => "yoda")
         v2 = v.merge("baz" => "qux")
-        v.vars.keys.should == ["foo"]
+        expect(v["foo"]).to eq("yoda")
-        v2.vars.keys.should =~ ["foo", "baz"]
+        expect(v2["foo"]).to eq("yoda")
+        expect(v2["baz"]).to eq("qux")
       end
+
       it "returns a new VarSet merged with the given VarSet" do
         v = VarSet.new("foo" => ["a", "b"], "bar" => 42)
         v2 = v.merge(VarSet.new("bar" => 33, "baz" => :baz))
         v2["foo"] << "c"
-        v["foo"].should == ["a", "b"]
+        expect(v["foo"]).to eq ["a", "b"]
-        v["bar"].should == 42
+        expect(v["bar"]).to eq 42
-        v2["foo"].should == ["a", "b", "c"]
+        expect(v2["foo"]).to eq ["a", "b", "c"]
-        v2["bar"].should == 33
+        expect(v2["bar"]).to eq 33
+      end
+
+      it "does not pick up subsequent variable changes from a given VarSet" do
+        v = VarSet.new("var" => ["a", "b"], "var2" => ["1", "2"])
+        v["var2"] << "3"
+        v2 = v.clone
+        v["var"] << "c"
+        expect(v["var"]).to eq(["a", "b", "c"])
+        expect(v["var2"]).to eq(["1", "2", "3"])
+        expect(v2["var"]).to eq(["a", "b"])
+        expect(v2["var2"]).to eq(["1", "2", "3"])
       end
     end