updated tests, fixed gc problem, and made locate not open up an element

ohler55 · Sep 29, 2013 · 5e488ff · 5e488ff
1 parent 5713afe
commit 5e488ff
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 35 deletions.
diff --git a/README.md b/README.md
@@ -34,10 +34,11 @@ A fast XML parser and Object marshaller as a Ruby gem.
 
 ## <a name="release">Release Notes</a>
 
-### Release 2.0.9
+### Release 2.0.10
 
- - Fixed bug that did not allow ISO-8859-1 characters and caused a crash.
+ - Small fix to not create an empty element from a closed element when using locate().
 
+ - Fixed to keep objects from being garbages collected in Ruby 2.x.
 
 ## <a name="description">Description</a>
 

diff --git a/ext/ox/extconf.rb b/ext/ox/extconf.rb
@@ -30,6 +30,7 @@
   'HAS_RSTRUCT' => ('ruby' == type || 'ree' == type) ? 1 : 0,
   'HAS_IVAR_HELPERS' => ('ruby' == type && ('1' == version[0] && '9' == version[1]) || '2' <= version[0]) ? 1 : 0,
   'HAS_PROC_WITH_BLOCK' => ('ruby' == type && ('1' == version[0] && '9' == version[1]) || '2' <= version[0]) ? 1 : 0,
+  'HAS_GC_GUARD' => ('jruby' != type && 'rubinius' != type) ? 1 : 0,
   'HAS_TOP_LEVEL_ST_H' => ('ree' == type || ('ruby' == type &&  '1' == version[0] && '8' == version[1])) ? 1 : 0,
   'NEEDS_UIO' => (RUBY_PLATFORM =~ /(win|w)32$/) ? 0 : 1,
 }

diff --git a/ext/ox/gen_load.c b/ext/ox/gen_load.c
@@ -94,6 +94,9 @@ create_doc(PInfo pi) {
 
     helper_stack_init(&pi->helpers);
     doc = rb_obj_alloc(ox_document_clas);
+#if HAS_GC_GUARD
+    RB_GC_GUARD(doc);
+#endif
     nodes = rb_ary_new();
     rb_ivar_set(doc, ox_attributes_id, rb_hash_new());
     rb_ivar_set(doc, ox_nodes_id, nodes);

diff --git a/ext/ox/ox.c b/ext/ox/ox.c
@@ -402,10 +402,17 @@ to_obj(VALUE self, VALUE ruby_xml) {
 	xml = ALLOCA_N(char, len);
     }
     memcpy(xml, x, len);
+#if HAS_GC_GUARD
+    rb_gc_disable();
+#endif
     obj = ox_parse(xml, ox_obj_callbacks, 0, &options, &err);
     if (SMALL_XML < len) {
 	xfree(xml);
     }
+#if HAS_GC_GUARD
+    RB_GC_GUARD(obj);
+    rb_gc_enable();
+#endif
     if (err_has(&err)) {
 	ox_err_raise(&err);
     }
@@ -514,7 +521,14 @@ load(char *xml, int argc, VALUE *argv, VALUE self, VALUE encoding, Err err) {
     xml = defuse_bom(xml, &options);
     switch (options.mode) {
     case ObjMode:
+#if HAS_GC_GUARD
+	rb_gc_disable();
+#endif
 	obj = ox_parse(xml, ox_obj_callbacks, 0, &options, err);
+#if HAS_GC_GUARD
+	RB_GC_GUARD(obj);
+	rb_gc_enable();
+#endif
 	break;
     case GenMode:
 	obj = ox_parse(xml, ox_gen_callbacks, 0, &options, err);
@@ -971,5 +985,8 @@ _ox_raise_error(const char *msg, const char *xml, const char *current, const cha
 	    xline++;
 	}
     }
+#if HAS_GC_GUARD
+    rb_gc_enable();
+#endif
     rb_raise(ox_parse_error_class, "%s at line %d, column %d [%s:%d]\n", msg, xline, col, file, line);
 }
diff --git a/lib/ox/element.rb b/lib/ox/element.rb
@@ -49,16 +49,16 @@ def initialize(name)
     # Element.
     # @return [Array] all child Nodes.
     def nodes
-      @nodes = [] if !instance_variable_defined?(:@nodes) or @nodes.nil?
+      return [] if !instance_variable_defined?(:@nodes) or @nodes.nil?
       @nodes
     end
 
     # Appends a Node to the Element's nodes array. Returns the element itself
     # so multiple appends can be chained together.
     # @param [Node] node Node to append to the nodes array
     def <<(node)
-      @nodes = [] if !instance_variable_defined?(:@nodes) or @nodes.nil?
       raise "argument to << must be a String or Ox::Node." unless node.is_a?(String) or node.is_a?(Node)
+      @nodes = [] if !instance_variable_defined?(:@nodes) or @nodes.nil?
       @nodes << node
       self
     end

diff --git a/lib/ox/version.rb b/lib/ox/version.rb
@@ -1,5 +1,5 @@
 
 module Ox
   # Current version of the module. 
-  VERSION = '2.0.9'
+  VERSION = '2.0.10'
 end
diff --git a/test/perf_gen.rb b/test/perf_gen.rb
@@ -86,28 +86,32 @@
   if do_load
     perf = Perf.new()
     perf.add('Ox', 'parse') { Ox.parse(xml) }
-    perf.add('Nokogiri', 'parse') { Nokogiri::XML::Document.parse(xml) }
+    perf.add('Nokogiri', 'parse') { Nokogiri::XML::Document.parse(xml) } unless defined?(::Nokogiri).nil?
-    perf.add('LibXML', 'parse') { LibXML::XML::Document.string(xml) }
+    perf.add('LibXML', 'parse') { LibXML::XML::Document.string(xml) } unless defined?(::LibXML).nil?
     perf.run($iter)
   end
 
   if do_dump
     perf = Perf.new()
     perf.add('Ox', 'dump') { Ox.dump($obj, :indent => 2) }
     perf.before('Ox') { $obj = d[:ox] }
-    perf.add('Nokogiri', 'dump') { $obj.to_xml(:indent => 2) }
+    unless defined?(::Nokogiri).nil?
-    perf.before('Nokogiri') { $obj = d[:nokogiri] }
+      perf.add('Nokogiri', 'dump') { $obj.to_xml(:indent => 2) }
-    perf.add('LibXML', 'dump') { $obj.to_s() }
+      perf.before('Nokogiri') { $obj = d[:nokogiri] }
-    perf.before('LibXML') { $obj = d[:libxml] }
+    end
+    unless defined?(::LibXML).nil?
+      perf.add('LibXML', 'dump') { $obj.to_s() }
+      perf.before('LibXML') { $obj = d[:libxml] }
+    end
     perf.run($iter)
   end
 
   if do_read
     $filename = d[:file]
     perf = Perf.new()
     perf.add('Ox', 'load_file') { Ox.load_file($filename, :mode => :generic) }
-    perf.add('Nokogiri', 'parse') { Nokogiri::XML::Document.parse(File.open($filename)) }
+    perf.add('Nokogiri', 'parse') { Nokogiri::XML::Document.parse(File.open($filename)) } unless defined?(::Nokogiri).nil?
-    perf.add('LibXML', 'parse') { LibXML::XML::Document.file($filename) }
+    perf.add('LibXML', 'parse') { LibXML::XML::Document.file($filename) } unless defined?(::LibXML).nil?
     perf.run($iter)
   end
 
@@ -116,16 +120,20 @@
     perf = Perf.new()
     perf.add('Ox', 'to_file') { Ox.to_file($filename, $obj, :indent => 0) }
     perf.before('Ox') { $obj = d[:ox] }
-    perf.add('Nokogiri', 'dump') {
+    unless defined?(::Nokogiri).nil?
-      xml = $obj.to_xml(:indent => 0)
+      perf.add('Nokogiri', 'dump') {
-      File.open($filename, "w") { |f| f.write(xml) }
+        xml = $obj.to_xml(:indent => 0)
-    }
+        File.open($filename, "w") { |f| f.write(xml) }
+      }
+    end
     perf.before('Nokogiri') { $obj = d[:nokogiri] }
-    perf.add('LibXML', 'dump') {
+    unless defined?(::LibXML).nil?
-      xml = $obj.to_s()
+      perf.add('LibXML', 'dump') {
-      File.open($filename, "w") { |f| f.write(xml) }
+        xml = $obj.to_s()
-    }
+        File.open($filename, "w") { |f| f.write(xml) }
-    perf.before('LibXML') { $obj = d[:libxml] }
+      }
+      perf.before('LibXML') { $obj = d[:libxml] }
+    end
     perf.run($iter)
   end
 

diff --git a/test/perf_obj.rb b/test/perf_obj.rb
@@ -13,10 +13,13 @@
 
 require 'optparse'
 require 'ox'
-require 'oj'
 require 'perf'
 require 'sample'
 require 'files'
+begin
+  require 'oj'
+rescue Exception
+end
 
 $circular = false
 $indent = 0
@@ -64,29 +67,30 @@
   $obj = do_sample ? sample_doc(2) : files('..')
   $mars = Marshal.dump($obj)
   $xml = Ox.dump($obj, :indent => $indent, :circular => $circular)
-  $json = Oj.dump($obj, :indent => $indent, :circular => $circular)
   File.open('sample.xml', 'w') { |f| f.write($xml) }
-  File.open('sample.json', 'w') { |f| f.write($json) }
   File.open('sample.marshal', 'w') { |f| f.write($mars) }
+  unless defined?(::Oj).nil?
+    $json = Oj.dump($obj, :indent => $indent, :circular => $circular) 
+    File.open('sample.json', 'w') { |f| f.write($json) }
+  end
 else
   puts "loading and parsing #{files}\n\n"
-  # TBD change to allow xml and json
+  files.map do |f|
-  data = files.map do |f|
     $xml = File.read(f)
     $obj = Ox.load($xml);
     $mars = Marshal.dump($obj)
-    $json = Oj.dump($obj, :indent => $indent, :circular => $circular)
+    $json = Oj.dump($obj, :indent => $indent, :circular => $circular)  unless defined?(::Oj).nil?
   end
 end
 
-Oj.default_options = { :mode => :object, :indent => $indent }
+Oj.default_options = { :mode => :object, :indent => $indent } unless defined?(::Oj).nil?
 
 if do_load
   puts '-' * 80
   puts "Load Performance"
   perf = Perf.new()
   perf.add('Ox', 'load') { Ox.load($xml, :mode => :object) }
-  perf.add('Oj', 'load') { Oj.load($json) }
+  perf.add('Oj', 'load') { Oj.load($json) } unless defined?(::Oj).nil?
   perf.add('Marshal', 'load') { Marshal.load($mars) }
   perf.run($iter)
 end
@@ -96,7 +100,7 @@
   puts "Dump Performance"
   perf = Perf.new()
   perf.add('Ox', 'dump') { Ox.dump($obj, :indent => $indent, :circular => $circular) }
-  perf.add('Oj', 'dump') { Oj.dump($obj) }
+  perf.add('Oj', 'dump') { Oj.dump($obj) } unless defined?(::Oj).nil?
   perf.add('Marshal', 'dump') { Marshal.dump($obj) }
   perf.run($iter)
 end
@@ -106,7 +110,7 @@
   puts "Read from file Performance"
   perf = Perf.new()
   perf.add('Ox', 'load_file') { Ox.load_file('sample.xml', :mode => :object) }
-  perf.add('Oj', 'load') { Oj.load_file('sample.json') }
+  perf.add('Oj', 'load') { Oj.load_file('sample.json') } unless defined?(::Oj).nil?
   perf.add('Marshal', 'load') { Marshal.load(File.new('sample.marshal')) }
   perf.run($iter)
 end
@@ -116,9 +120,7 @@
   puts "Write to file Performance"
   perf = Perf.new()
   perf.add('Ox', 'to_file') { Ox.to_file('sample.xml', $obj, :indent => $indent, :circular => $circular) }
-  perf.add('Oj', 'to_file') { Oj.to_file('sample.json', $obj) }
+  perf.add('Oj', 'to_file') { Oj.to_file('sample.json', $obj) } unless defined?(::Oj).nil?
   perf.add('Marshal', 'dump') { Marshal.dump($obj, File.new('sample.marshal', 'w')) }
   perf.run($iter)
 end
-
-