From 5ce39e0a71c94b5226326c6985dea5888a75a413 Mon Sep 17 00:00:00 2001 From: chopraanmol1 Date: Fri, 11 Jan 2019 12:25:40 +0530 Subject: [PATCH 1/2] Add finalizer to weak instance cache --- lib/roo/helpers/weak_instance_cache.rb | 9 ++++ spec/lib/roo/weak_instance_cache_spec.rb | 69 ++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 spec/lib/roo/weak_instance_cache_spec.rb diff --git a/lib/roo/helpers/weak_instance_cache.rb b/lib/roo/helpers/weak_instance_cache.rb index 96c026a2..bc853877 100644 --- a/lib/roo/helpers/weak_instance_cache.rb +++ b/lib/roo/helpers/weak_instance_cache.rb @@ -22,11 +22,20 @@ def instance_cache(key) unless object object = yield + ObjectSpace.define_finalizer(object, instance_cache_finalizer(key)) instance_variable_set(key, WeakRef.new(object)) end object end + + def instance_cache_finalizer(key) + proc do + if instance_variable_defined?(key) && (ref = instance_variable_get(key)) && !ref.weakref_alive? + remove_instance_variable(key) + end + end + end end end end diff --git a/spec/lib/roo/weak_instance_cache_spec.rb b/spec/lib/roo/weak_instance_cache_spec.rb new file mode 100644 index 00000000..cc2caf46 --- /dev/null +++ b/spec/lib/roo/weak_instance_cache_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe Roo::Helpers::WeakInstanceCache do + let(:klass) do + Class.new do + include Roo::Helpers::WeakInstanceCache + + def memoized_data + instance_cache(:@memoized_data) do + "Some Costly Operation" * 1_000 + end + end + end + end + + subject do + klass.new + end + + it 'should be lazy' do + expect(subject.instance_variables).to_not include(:@memoized_data) + data = subject.memoized_data + expect(subject.instance_variables).to include(:@memoized_data) + end + + + it 'should be memoized' do + data = subject.memoized_data + expect(subject.memoized_data).to equal(data) + end + + it 'should recalculate after GC' do + GC.disable + original_id = subject.memoized_data.object_id + expect(subject.memoized_data.object_id).to eq(original_id) + + GC.start + expect(subject.memoized_data.object_id).not_to eq(original_id) + end + + it 'must remove instance variable' do + expect(subject.instance_variables).to_not include(:@memoized_data) + GC.disable + subject.memoized_data + expect(subject.instance_variables).to include(:@memoized_data) + + GC.start + expect(subject.instance_variables).to_not include(:@memoized_data) + end + + context '#inspect must not raise' do + it 'before calculation' do + expect{subject.inspect}.to_not raise_error + end + it 'after calculation' do + GC.disable + subject.memoized_data + expect{subject.inspect}.to_not raise_error + expect(subject.inspect).to include("Some Costly Operation") + GC.start + end + it 'after GC' do + subject.memoized_data + GC.start + expect{subject.inspect}.to_not raise_error + expect(subject.inspect).to_not include("Some Costly Operation") + end + end +end \ No newline at end of file From ddcf495c58e377fe21debb931740da1a86093f79 Mon Sep 17 00:00:00 2001 From: chopraanmol1 Date: Fri, 11 Jan 2019 13:54:09 +0530 Subject: [PATCH 2/2] Fix Jruby spec --- lib/roo/helpers/weak_instance_cache.rb | 4 +-- spec/lib/roo/weak_instance_cache_spec.rb | 43 ++++++++++++++++++------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/lib/roo/helpers/weak_instance_cache.rb b/lib/roo/helpers/weak_instance_cache.rb index bc853877..db26de1a 100644 --- a/lib/roo/helpers/weak_instance_cache.rb +++ b/lib/roo/helpers/weak_instance_cache.rb @@ -30,8 +30,8 @@ def instance_cache(key) end def instance_cache_finalizer(key) - proc do - if instance_variable_defined?(key) && (ref = instance_variable_get(key)) && !ref.weakref_alive? + proc do |object_id| + if instance_variable_defined?(key) && (ref = instance_variable_get(key)) && (!ref.weakref_alive? || ref.__getobj__.object_id == object_id) remove_instance_variable(key) end end diff --git a/spec/lib/roo/weak_instance_cache_spec.rb b/spec/lib/roo/weak_instance_cache_spec.rb index cc2caf46..5adcb908 100644 --- a/spec/lib/roo/weak_instance_cache_spec.rb +++ b/spec/lib/roo/weak_instance_cache_spec.rb @@ -1,5 +1,10 @@ require 'spec_helper' +if RUBY_PLATFORM == "java" + require 'java' + java_import 'java.lang.System' +end + describe Roo::Helpers::WeakInstanceCache do let(:klass) do Class.new do @@ -7,7 +12,7 @@ def memoized_data instance_cache(:@memoized_data) do - "Some Costly Operation" * 1_000 + "Some Costly Operation #{rand(1000)}" * 1_000 end end end @@ -30,21 +35,25 @@ def memoized_data end it 'should recalculate after GC' do + expect(subject.instance_variables).to_not include(:@memoized_data) GC.disable - original_id = subject.memoized_data.object_id - expect(subject.memoized_data.object_id).to eq(original_id) + subject.memoized_data && nil + expect(subject.instance_variables).to include(:@memoized_data) - GC.start - expect(subject.memoized_data.object_id).not_to eq(original_id) + force_gc + expect(subject.instance_variables).to_not include(:@memoized_data) + GC.disable + subject.memoized_data && nil + expect(subject.instance_variables).to include(:@memoized_data) end it 'must remove instance variable' do expect(subject.instance_variables).to_not include(:@memoized_data) GC.disable - subject.memoized_data + subject.memoized_data && nil expect(subject.instance_variables).to include(:@memoized_data) - GC.start + force_gc expect(subject.instance_variables).to_not include(:@memoized_data) end @@ -54,16 +63,28 @@ def memoized_data end it 'after calculation' do GC.disable - subject.memoized_data + subject.memoized_data && nil expect{subject.inspect}.to_not raise_error expect(subject.inspect).to include("Some Costly Operation") - GC.start + force_gc end it 'after GC' do - subject.memoized_data - GC.start + subject.memoized_data && nil + force_gc + expect(subject.instance_variables).to_not include(:@memoized_data) expect{subject.inspect}.to_not raise_error expect(subject.inspect).to_not include("Some Costly Operation") end end + + if RUBY_PLATFORM == "java" + def force_gc + System.gc + sleep(0.1) + end + else + def force_gc + GC.start + end + end end \ No newline at end of file