Reduce Memory Size of Roo::Excelx::Cell::* classes

One of the factor which determines the memsize of object is total no. of instance variables Reducing no. of instance variables can also reduce memsize. To do so I've done two major things 1. Remove unnecessary instance_variable link 2. Created method attr_reader_with_default i. With help of this method we can remove instance variable which are static accross class e.g. type and in somecases cell_type ii. We can use better default to reduce memory e.g. style Script ``` require 'roo' require 'objspace' coordinate = Roo::Excelx::Coordinate.new 1,1 base_date = Date.new(1899, 12, 30) cells = [] formulas = [nil, "something"]#.reverse styles = [1,2]#.reverse # Changing the sequence of this will result into different result formulas.each do |formula| styles.each do |style| cells << ["formula=#{formula};style=#{style}", Roo::Excelx::Cell::String.new('Sample String',formula, style, nil, coordinate)] cells << ["formula=#{formula};style=#{style}", Roo::Excelx::Cell::Boolean.new('1',formula, style,nil,coordinate)] cells << ["formula=#{formula};style=#{style}", Roo::Excelx::Cell::Number.new('20.25', formula, [:numeric_or_formula, '%.2f'], style, nil, coordinate)] cells << ["formula=#{formula};style=#{style}", Roo::Excelx::Cell::Date.new(5, formula, [:numeric_or_formula, 'yyyy-mm-ddd'], style,nil,base_date,coordinate)] cells << ["formula=#{formula};style=#{style}", Roo::Excelx::Cell::DateTime.new(5.25, formula, [:numeric_or_formula, 'yyyy-mm-ddd hh:mm'], style,nil,base_date,coordinate)] cells << ["formula=#{formula};style=#{style}", Roo::Excelx::Cell::Time.new(0.25, formula, [:numeric_or_formula, 'hh:mm'], style,nil,base_date,coordinate)] cells << ["", Roo::Excelx::Cell::Empty.new(coordinate)] end end cells = cells.collect{|variant,cell| ["#{cell.class}(#{variant})",cell] }.uniq(&:first).sort_by(&:first) cells.each do |text, cell| puts "#{text} => #{ObjectSpace.memsize_of(cell)} (#{cell.instance_variables.length})" end ``` Result on Master ``` Roo::Excelx::Cell::Boolean(formula=;style=1) => 120 (8) Roo::Excelx::Cell::Boolean(formula=;style=2) => 104 (8) Roo::Excelx::Cell::Boolean(formula=something;style=1) => 104 (8) Roo::Excelx::Cell::Boolean(formula=something;style=2) => 104 (8) Roo::Excelx::Cell::Date(formula=;style=1) => 120 (9) Roo::Excelx::Cell::Date(formula=;style=2) => 112 (9) Roo::Excelx::Cell::Date(formula=something;style=1) => 112 (9) Roo::Excelx::Cell::Date(formula=something;style=2) => 112 (9) Roo::Excelx::Cell::DateTime(formula=;style=1) => 120 (9) Roo::Excelx::Cell::DateTime(formula=;style=2) => 112 (9) Roo::Excelx::Cell::DateTime(formula=something;style=1) => 112 (9) Roo::Excelx::Cell::DateTime(formula=something;style=2) => 112 (9) Roo::Excelx::Cell::Empty() => 96 (7) Roo::Excelx::Cell::Number(formula=;style=1) => 120 (9) Roo::Excelx::Cell::Number(formula=;style=2) => 112 (9) Roo::Excelx::Cell::Number(formula=something;style=1) => 112 (9) Roo::Excelx::Cell::Number(formula=something;style=2) => 112 (9) Roo::Excelx::Cell::String(formula=;style=1) => 120 (8) Roo::Excelx::Cell::String(formula=;style=2) => 104 (8) Roo::Excelx::Cell::String(formula=something;style=1) => 104 (8) Roo::Excelx::Cell::String(formula=something;style=2) => 104 (8) Roo::Excelx::Cell::Time(formula=;style=1) => 120 (10) Roo::Excelx::Cell::Time(formula=;style=2) => 120 (10) Roo::Excelx::Cell::Time(formula=something;style=1) => 120 (10) Roo::Excelx::Cell::Time(formula=something;style=2) => 120 (10) ``` Result after this patch ``` Roo::Excelx::Cell::Boolean(formula=;style=1) => 40 (3) Roo::Excelx::Cell::Boolean(formula=;style=2) => 80 (4) Roo::Excelx::Cell::Boolean(formula=something;style=1) => 88 (4) Roo::Excelx::Cell::Boolean(formula=something;style=2) => 80 (5) Roo::Excelx::Cell::Date(formula=;style=1) => 80 (5) Roo::Excelx::Cell::Date(formula=;style=2) => 96 (6) Roo::Excelx::Cell::Date(formula=something;style=1) => 104 (6) Roo::Excelx::Cell::Date(formula=something;style=2) => 96 (7) Roo::Excelx::Cell::DateTime(formula=;style=1) => 80 (5) Roo::Excelx::Cell::DateTime(formula=;style=2) => 96 (6) Roo::Excelx::Cell::DateTime(formula=something;style=1) => 104 (6) Roo::Excelx::Cell::DateTime(formula=something;style=2) => 96 (7) Roo::Excelx::Cell::Empty() => 40 (1) Roo::Excelx::Cell::Number(formula=;style=1) => 80 (5) Roo::Excelx::Cell::Number(formula=;style=2) => 96 (6) Roo::Excelx::Cell::Number(formula=something;style=1) => 104 (6) Roo::Excelx::Cell::Number(formula=something;style=2) => 96 (7) Roo::Excelx::Cell::String(formula=;style=1) => 40 (3) Roo::Excelx::Cell::String(formula=;style=2) => 80 (4) Roo::Excelx::Cell::String(formula=something;style=1) => 88 (4) Roo::Excelx::Cell::String(formula=something;style=2) => 80 (5) Roo::Excelx::Cell::Time(formula=;style=1) => 96 (6) Roo::Excelx::Cell::Time(formula=;style=2) => 104 (7) Roo::Excelx::Cell::Time(formula=something;style=1) => 120 (7) Roo::Excelx::Cell::Time(formula=something;style=2) => 104 (8) ```
roo-rb · Sep 16, 2018 · ca31e0e · ca31e0e
1 parent 4f9b166
commit ca31e0e
Show file tree

Hide file tree

Showing 11 changed files with 129 additions and 28 deletions.
diff --git a/lib/roo/attr_reader_helper.rb b/lib/roo/attr_reader_helper.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Roo
+  module AttrReaderHelper
+    def attr_reader_with_default(attr_hash)
+      attr_hash.each do |attr_name, default_value|
+        instance_variable = :"@#{attr_name}"
+        define_method attr_name do
+          if instance_variable_defined? instance_variable
+            instance_variable_get instance_variable
+          else
+            default_value
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/roo/excelx/cell/base.rb b/lib/roo/excelx/cell/base.rb
@@ -1,13 +1,18 @@
+# frozen_string_literal: true
+
+require "roo/attr_reader_helper"
+
 module Roo
   class Excelx
     class Cell
       class Base
+        extend Roo::AttrReaderHelper
         attr_reader :cell_type, :cell_value, :value
 
         # FIXME: I think style should be deprecated. Having a style attribute
         #        for a cell doesn't really accomplish much. It seems to be used
         #        when you want to export to excelx.
-        attr_reader :style
+        attr_reader_with_default default_type: :base, style: 1
 
 
         # FIXME: Updating a cell's value should be able tochange the cell's type,
@@ -34,14 +39,12 @@ class Base
         attr_writer :value
 
         def initialize(value, formula, excelx_type, style, link, coordinate)
-          @link = !!link
           @cell_value = value
-          @cell_type = excelx_type
-          @formula = formula
-          @style = style
+          @cell_type = excelx_type if excelx_type
+          @formula = formula if formula
+          @style = style unless style == 1
           @coordinate = coordinate
-          @type = :base
-          @value = link? ? Roo::Link.new(link, value) : value
+          @value = link ? Roo::Link.new(link, value) : value
         end
 
         def type
@@ -50,7 +53,7 @@ def type
           elsif link?
             :link
           else
-            @type
+            default_type
           end
         end
 
@@ -59,7 +62,7 @@ def formula?
         end
 
         def link?
-          !!@link
+          Roo::Link === @value
         end
 
         alias_method :formatted_value, :value

diff --git a/lib/roo/excelx/cell/boolean.rb b/lib/roo/excelx/cell/boolean.rb
@@ -4,12 +4,13 @@ module Roo
   class Excelx
     class Cell
       class Boolean < Cell::Base
-        attr_reader :value, :formula, :format, :cell_type, :cell_value, :link, :coordinate
+        attr_reader :value, :formula, :format, :cell_value, :link, :coordinate
+
+        attr_reader_with_default default_type: :boolean, cell_type: :boolean
 
         def initialize(value, formula, style, link, coordinate)
-          super(value, formula, nil, style, link, coordinate)
-          @type = @cell_type = :boolean
-          @value = link? ? Roo::Link.new(link, value) : create_boolean(value)
+          super(value, formula, nil, style, nil, coordinate)
+          @value = link ? Roo::Link.new(link, value) : create_boolean(value)
         end
 
         def formatted_value
@@ -21,7 +22,7 @@ def formatted_value
         def create_boolean(value)
           # FIXME: Using a boolean will cause methods like Base#to_csv to fail.
           #       Roo is using some method to ignore false/nil values.
-          value.to_i == 1 ? true : false
+          value.to_i == 1
         end
       end
     end

diff --git a/lib/roo/excelx/cell/date.rb b/lib/roo/excelx/cell/date.rb
@@ -6,12 +6,13 @@ class Cell
       class Date < Roo::Excelx::Cell::DateTime
         attr_reader :value, :formula, :format, :cell_type, :cell_value, :link, :coordinate
 
+        attr_reader_with_default default_type: :date
+
         def initialize(value, formula, excelx_type, style, link, base_date, coordinate)
           # NOTE: Pass all arguments to the parent class, DateTime.
           super
-          @type = :date
           @format = excelx_type.last
-          @value = link? ? Roo::Link.new(link, value) : create_date(base_date, value)
+          @value = link ? Roo::Link.new(link, value) : create_date(base_date, value)
         end
 
         private

diff --git a/lib/roo/excelx/cell/datetime.rb b/lib/roo/excelx/cell/datetime.rb
@@ -10,11 +10,12 @@ class DateTime < Cell::Base
 
         attr_reader :value, :formula, :format, :cell_value, :link, :coordinate
 
+        attr_reader_with_default default_type: :datetime
+
         def initialize(value, formula, excelx_type, style, link, base_timestamp, coordinate)
-          super(value, formula, excelx_type, style, link, coordinate)
-          @type = :datetime
+          super(value, formula, excelx_type, style, nil, coordinate)
           @format = excelx_type.last
-          @value = link? ? Roo::Link.new(link, value) : create_datetime(base_timestamp, value)
+          @value = link ? Roo::Link.new(link, value) : create_datetime(base_timestamp, value)
         end
 
         # Public: Returns formatted value for a datetime. Format's can be an

diff --git a/lib/roo/excelx/cell/empty.rb b/lib/roo/excelx/cell/empty.rb
@@ -5,8 +5,9 @@ class Cell
       class Empty < Cell::Base
         attr_reader :value, :formula, :format, :cell_type, :cell_value, :hyperlink, :coordinate
 
+        attr_reader_with_default default_type: nil, style: nil
+
         def initialize(coordinate)
-          @value = @formula = @format = @cell_type = @cell_value = @hyperlink = nil
           @coordinate = coordinate
         end
 

diff --git a/lib/roo/excelx/cell/number.rb b/lib/roo/excelx/cell/number.rb
@@ -6,13 +6,14 @@ class Cell
       class Number < Cell::Base
         attr_reader :value, :formula, :format, :cell_value, :link, :coordinate
 
+        # FIXME: change default_type to number. This will break brittle tests.
+        attr_reader_with_default default_type: :float
+
         def initialize(value, formula, excelx_type, style, link, coordinate)
           super
-          # FIXME: change @type to number. This will break brittle tests.
           # FIXME: Excelx_type is an array, but the first value isn't used.
-          @type = :float
           @format = excelx_type.last
-          @value = link? ? Roo::Link.new(link, value) : create_numeric(value)
+          @value = link ? Roo::Link.new(link, value) : create_numeric(value)
         end
 
         def create_numeric(number)

diff --git a/lib/roo/excelx/cell/string.rb b/lib/roo/excelx/cell/string.rb
@@ -2,12 +2,12 @@ module Roo
   class Excelx
     class Cell
       class String < Cell::Base
-        attr_reader :value, :formula, :format, :cell_type, :cell_value, :link, :coordinate
+        attr_reader :value, :formula, :format, :cell_value, :link, :coordinate
+
+        attr_reader_with_default default_type: :string, cell_type: :string
 
         def initialize(value, formula, style, link, coordinate)
           super(value, formula, nil, style, link, coordinate)
-          @type = @cell_type = :string
-          @value = link? ? Roo::Link.new(link, value) : value
         end
 
         def empty?

diff --git a/lib/roo/excelx/cell/time.rb b/lib/roo/excelx/cell/time.rb
@@ -6,13 +6,14 @@ class Cell
       class Time < Roo::Excelx::Cell::DateTime
         attr_reader :value, :formula, :format, :cell_value, :link, :coordinate
 
+        attr_reader_with_default default_type: :time
+
         def initialize(value, formula, excelx_type, style, link, base_date, coordinate)
           # NOTE: Pass all arguments to DateTime super class.
           super
-          @type = :time
           @format = excelx_type.last
           @datetime = create_datetime(base_date, value)
-          @value = link? ? Roo::Link.new(link, value) : (value.to_f * 86_400).to_i
+          @value = link ? Roo::Link.new(link, value) : (value.to_f * 86_400).to_i
         end
 
         def formatted_value

diff --git a/lib/roo/excelx/sheet_doc.rb b/lib/roo/excelx/sheet_doc.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'forwardable'
 require 'roo/excelx/extractor'
 

diff --git a/test/excelx/cell/test_attr_reader_default.rb b/test/excelx/cell/test_attr_reader_default.rb
@@ -0,0 +1,72 @@
+require "test_helper"
+
+class TestAttrReaderDefault < Minitest::Test
+  def base
+    Roo::Excelx::Cell::Base
+  end
+
+  def boolean
+    Roo::Excelx::Cell::Boolean
+  end
+
+  def class_date
+    Roo::Excelx::Cell::Date
+  end
+
+  def datetime
+    Roo::Excelx::Cell::DateTime
+  end
+
+  def empty
+    Roo::Excelx::Cell::Empty
+  end
+
+  def number
+    Roo::Excelx::Cell::Number
+  end
+
+  def string
+    Roo::Excelx::Cell::String
+  end
+
+  def base_date
+    ::Date.new(1899, 12, 30)
+  end
+
+  def base_timestamp
+    ::Date.new(1899, 12, 30).to_datetime.to_time.to_i
+  end
+
+  def class_time
+    Roo::Excelx::Cell::Time
+  end
+
+  def test_cell_default_values
+    assert_values base.new(nil, nil, [], 1, nil, nil), default_type: :base, :@default_type => nil, style: 1, :@style => nil
+    assert_values boolean.new("1", nil, nil, nil, nil), default_type: :boolean, :@default_type => nil, cell_type: :boolean, :@cell_type => nil
+    assert_values class_date.new("41791", nil, [:numeric_or_formula, "mm-dd-yy"], 6, nil, base_date, nil), default_type: :date, :@default_type => nil
+    assert_values class_time.new("0.521", nil, [:numeric_or_formula, "hh:mm"], 6, nil, base_timestamp, nil), default_type: :time, :@default_type => nil
+    assert_values datetime.new("41791.521", nil, [:numeric_or_formula, "mm-dd-yy hh:mm"], 6, nil, base_timestamp, nil), default_type: :datetime, :@default_type => nil
+    assert_values empty.new(nil), default_type: nil, :@default_type => nil, style: nil, :@style => nil
+    assert_values number.new("42", nil, ["0"], nil, nil, nil), default_type: :float, :@default_type => nil
+    assert_values string.new("1", nil, nil, nil, nil), default_type: :string, :@default_type => nil, cell_type: :string, :@cell_type => nil
+
+    assert_values base.new(nil, nil, [], 2, nil, nil), style: 2, :@style => 2
+  end
+
+  def assert_values(object, value_hash)
+    value_hash.each do |attr_name, expected_value|
+      value = if attr_name.to_s.include?("@")
+                object.instance_variable_get(attr_name)
+              else
+                object.public_send(attr_name)
+      end
+
+      if expected_value
+        assert_equal expected_value, value
+      else
+        assert_nil value
+      end
+    end
+  end
+end