Merge pull request #171 from ojab/lil_speedup

Fixup benchmark and speedup a little, fixes #141

Gemfile

@@ -5,6 +5,7 @@ source 'https://rubygems.org/'
 gemspec
 
 group :benchmark do
+  gem 'benchmark-ips'
   gem 'kramdown'
   gem 'redcarpet'
 end

bin/commonmarker

@@ -2,7 +2,6 @@
 # frozen_string_literal: true
 
 require 'optparse'
-require 'ostruct'
 
 $LOAD_PATH.unshift File.join(File.dirname(__FILE__), '..', 'lib')
 require 'commonmarker'
@@ -11,17 +10,13 @@ root = File.expand_path('..', __dir__)
 $LOAD_PATH.unshift File.expand_path('lib', root)
 
 def parse_options
-  options = OpenStruct.new
+  options = Struct.new(:active_extensions, :active_parse_options, :active_render_options, :output_format, :renderer)
+                  .new([], [:DEFAULT], [:DEFAULT], :html)
   extensions = CommonMarker.extensions
   parse_options = CommonMarker::Config::OPTS.fetch(:parse)
   render_options = CommonMarker::Config::OPTS.fetch(:render)
   format_options = CommonMarker::Config::OPTS.fetch(:format)
 
-  options.active_extensions = []
-  options.active_parse_options = [:DEFAULT]
-  options.active_render_options = [:DEFAULT]
-  options.output_format = :html
-
   option_parser = OptionParser.new do |opts|
     opts.banner = 'Usage: commonmarker [--html-renderer] [--extension=EXTENSION]'
     opts.separator '                    [--to=FORMAT]'

commonmarker.gemspec

@@ -23,6 +23,8 @@ Gem::Specification.new do |s|
   s.require_paths = %w[lib ext]
   s.required_ruby_version = ['>= 2.6', '< 4.0']
 
+  s.metadata['rubygems_mfa_required'] = 'true'
+
   s.rdoc_options += ['-x', 'ext/commonmarker/cmark/.*']
 
   s.add_development_dependency 'awesome_print'

ext/commonmarker/commonmarker.c

@@ -115,25 +115,23 @@ static void rb_parent_removed(VALUE val) {
   RDATA(val)->dfree = rb_free_c_struct;
 }
 
-static cmark_parser *prepare_parser(VALUE rb_options, VALUE rb_extensions, cmark_mem *mem) {
+static cmark_parser *prepare_parser(VALUE rb_options, VALUE rb_extensions) {
   int options;
-  int extensions_len;
   VALUE rb_ext_name;
   int i;
 
-  Check_Type(rb_options, T_FIXNUM);
+  FIXNUM_P(rb_options);
+  options = FIX2INT(rb_options);
+
   Check_Type(rb_extensions, T_ARRAY);
 
-  options = FIX2INT(rb_options);
-  extensions_len = RARRAY_LEN(rb_extensions);
+  cmark_parser *parser = cmark_parser_new(options);
 
-  cmark_parser *parser = cmark_parser_new_with_mem(options, mem);
-  for (i = 0; i < extensions_len; ++i) {
-    rb_ext_name = RARRAY_PTR(rb_extensions)[i];
+  for (i = 0; i < RARRAY_LEN(rb_extensions); ++i) {
+    rb_ext_name = rb_ary_entry(rb_extensions, i);
 
     if (!SYMBOL_P(rb_ext_name)) {
       cmark_parser_free(parser);
-      cmark_arena_reset();
       rb_raise(rb_eTypeError, "extension names should be Symbols; got a %"PRIsVALUE"", rb_obj_class(rb_ext_name));
     }
 
@@ -142,7 +140,6 @@ static cmark_parser *prepare_parser(VALUE rb_options, VALUE rb_extensions, cmark
 
     if (!syntax_extension) {
       cmark_parser_free(parser);
-      cmark_arena_reset();
       rb_raise(rb_eArgError, "extension %s not found", rb_id2name(SYM2ID(rb_ext_name)));
     }
 
@@ -157,67 +154,57 @@ static cmark_parser *prepare_parser(VALUE rb_options, VALUE rb_extensions, cmark
  *
  */
 static VALUE rb_markdown_to_html(VALUE self, VALUE rb_text, VALUE rb_options, VALUE rb_extensions) {
-  char *str, *html;
-  int len;
+  char *html;
   cmark_parser *parser;
   cmark_node *doc;
-  Check_Type(rb_text, T_STRING);
-  Check_Type(rb_options, T_FIXNUM);
 
-  parser = prepare_parser(rb_options, rb_extensions, cmark_get_arena_mem_allocator());
+  Check_Type(rb_text, T_STRING);
 
-  str = (char *)RSTRING_PTR(rb_text);
-  len = RSTRING_LEN(rb_text);
+  parser = prepare_parser(rb_options, rb_extensions);
 
-  cmark_parser_feed(parser, str, len);
+  cmark_parser_feed(parser, StringValuePtr(rb_text), RSTRING_LEN(rb_text));
   doc = cmark_parser_finish(parser);
+
   if (doc == NULL) {
-    cmark_arena_reset();
+    cmark_parser_free(parser);
     rb_raise(rb_eNodeError, "error parsing document");
   }
 
-  cmark_mem *default_mem = cmark_get_default_mem_allocator();
-  html = cmark_render_html_with_mem(doc, FIX2INT(rb_options), parser->syntax_extensions, default_mem);
-  cmark_arena_reset();
+  html = cmark_render_html(doc, parser->options, parser->syntax_extensions);
 
-  VALUE ruby_html = rb_str_new2(html);
-  default_mem->free(html);
+  cmark_parser_free(parser);
+  cmark_node_free(doc);
 
-  return ruby_html;
+  return rb_utf8_str_new_cstr(html);
 }
 
 /*
  * Internal: Parses a Markdown string into an HTML string.
  *
  */
 static VALUE rb_markdown_to_xml(VALUE self, VALUE rb_text, VALUE rb_options, VALUE rb_extensions) {
-  char *str, *xml;
-  int len;
+  char *xml;
   cmark_parser *parser;
   cmark_node *doc;
-  Check_Type(rb_text, T_STRING);
-  Check_Type(rb_options, T_FIXNUM);
 
-  parser = prepare_parser(rb_options, rb_extensions, cmark_get_arena_mem_allocator());
+  Check_Type(rb_text, T_STRING);
 
-  str = (char *)RSTRING_PTR(rb_text);
-  len = RSTRING_LEN(rb_text);
+  parser = prepare_parser(rb_options, rb_extensions);
 
-  cmark_parser_feed(parser, str, len);
+  cmark_parser_feed(parser, StringValuePtr(rb_text), RSTRING_LEN(rb_text));
   doc = cmark_parser_finish(parser);
+
   if (doc == NULL) {
-    cmark_arena_reset();
+    cmark_parser_free(parser);
     rb_raise(rb_eNodeError, "error parsing document");
   }
 
-  cmark_mem *default_mem = cmark_get_default_mem_allocator();
-  xml = cmark_render_xml_with_mem(doc, FIX2INT(rb_options), default_mem);
-  cmark_arena_reset();
+  xml = cmark_render_xml(doc, parser->options);
 
-  VALUE ruby_xml = rb_str_new2(xml);
-  default_mem->free(xml);
+  cmark_parser_free(parser);
+  cmark_node_free(doc);
 
-  return ruby_xml;
+  return rb_utf8_str_new_cstr(xml);
 }
 
 /*
@@ -308,18 +295,17 @@ static VALUE rb_node_new(VALUE self, VALUE type) {
 static VALUE rb_parse_document(VALUE self, VALUE rb_text, VALUE rb_len,
                                VALUE rb_options, VALUE rb_extensions) {
   char *text;
-  int len, options;
+  int len;
   cmark_parser *parser;
   cmark_node *doc;
   Check_Type(rb_text, T_STRING);
   Check_Type(rb_len, T_FIXNUM);
   Check_Type(rb_options, T_FIXNUM);
 
-  parser = prepare_parser(rb_options, rb_extensions, cmark_get_default_mem_allocator());
+  parser = prepare_parser(rb_options, rb_extensions);
 
   text = (char *)RSTRING_PTR(rb_text);
   len = FIX2INT(rb_len);
-  options = FIX2INT(rb_options);
 
   cmark_parser_feed(parser, text, len);
   doc = cmark_parser_finish(parser);
@@ -614,7 +600,6 @@ static VALUE rb_render_html(VALUE self, VALUE rb_options, VALUE rb_extensions) {
  */
 static VALUE rb_render_xml(VALUE self, VALUE rb_options) {
   int options;
-  int i;
   cmark_node *node;
   Check_Type(rb_options, T_FIXNUM);
 

lib/commonmarker.rb

@@ -23,9 +23,7 @@ def self.render_html(text, options = :DEFAULT, extensions = [])
     raise TypeError, "text must be a String; got a #{text.class}!" unless text.is_a?(String)
 
     opts = Config.process_options(options, :render)
-    text = text.encode('UTF-8')
-    html = Node.markdown_to_html(text, opts, extensions)
-    html.force_encoding('UTF-8')
+    Node.markdown_to_html(text.encode('UTF-8'), opts, extensions)
   end
 
   # Public: Parses a Markdown string into a `document` node.

test/benchmark.rb

@@ -1,32 +1,39 @@
 # frozen_string_literal: true
 
+require 'benchmark/ips'
 require 'commonmarker'
-require 'github/markdown'
 require 'redcarpet'
 require 'kramdown'
 require 'benchmark'
 
-def dobench(name, &blk)
-  puts name
-  puts Benchmark.measure(&blk)
-end
+benchinput = File.read('test/benchinput.md').freeze
 
-benchinput = File.open('test/benchinput.md', 'r').read
+printf("input size = %<bytes>d bytes\n\n", { bytes: benchinput.bytesize })
 
-printf("input size = %<bytes>d bytes\n\n", benchinput.bytesize)
+Benchmark.ips do |x|
+  x.report('redcarpet') do
+    Redcarpet::Markdown.new(Redcarpet::Render::HTML, autolink: false, tables: false).render(benchinput)
+  end
 
-dobench('redcarpet') do
-  Redcarpet::Markdown.new(Redcarpet::Render::HTML, autolink: false, tables: false).render(benchinput)
-end
+  x.report('commonmarker with to_html') do
+    CommonMarker.render_html(benchinput)
+  end
 
-dobench('commonmarker with to_html') do
-  CommonMarker.render_html(benchinput)
-end
+  x.report('commonmarker with to_xml') do
+    CommonMarker.render_html(benchinput)
+  end
 
-dobench('commonmarker with ruby HtmlRenderer') do
-  CommonMarker::HtmlRenderer.new.render(CommonMarker.render_doc(benchinput))
-end
+  x.report('commonmarker with ruby HtmlRenderer') do
+    CommonMarker::HtmlRenderer.new.render(CommonMarker.render_doc(benchinput))
+  end
+
+  x.report('commonmarker with render_doc.to_html') do
+    CommonMarker.render_doc(benchinput, :DEFAULT, [:autolink]).to_html(:DEFAULT, [:autolink])
+  end
+
+  x.report('kramdown') do
+    Kramdown::Document.new(benchinput).to_html(benchinput)
+  end
 
-dobench('kramdown') do
-  Kramdown::Document.new(benchinput).to_html(benchinput)
+  x.compare!
 end