Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace thread_safe with concurrent-ruby #2822

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ Improvements::
* use shorthands %F and %T instead of %Y-%m-%d and %H:%M:%S to format time
* read file in binary mode whenever contents are being normalized

Build / Infrastructure::

* replace thread_safe with concurrent-ruby (PR #2822) (*@junaruga*)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part should be changed? This PR is not only for Build / Infrastructure?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it changes the APIs used by this project, you're right. It's not just a dependency change. Therefore, it should now be under Improvements.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I moved it to Improvement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

// tag::compact[]
== 1.5.7.1 (2018-05-10) - @mojavelinux

Expand Down
31 changes: 18 additions & 13 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,30 @@ source 'https://rubygems.org'
gemspec

group :development do
if (ruby_version = Gem::Version.new RUBY_VERSION) < (Gem::Version.new '2.1.0')
if ruby_version < (Gem::Version.new '2.0.0')
gem 'haml', '~> 4.0.0'
if ruby_version < (Gem::Version.new '1.9.3')
gem 'cucumber', '~> 1.3.0'
gem 'nokogiri', '~> 1.5.0'
gem 'slim', '~> 2.1.0'
gem 'tilt', '2.0.7'
ruby_version = Gem::Version.new RUBY_VERSION
gem 'concurrent-ruby', '~> 1.0.0' unless ruby_version < (Gem::Version.new '1.9.3')
if ruby_version < (Gem::Version.new '2.2.0')
if ruby_version < (Gem::Version.new '2.1.0')
if ruby_version < (Gem::Version.new '2.0.0')
gem 'haml', '~> 4.0.0'
if ruby_version < (Gem::Version.new '1.9.3')
gem 'cucumber', '~> 1.3.0'
gem 'nokogiri', '~> 1.5.0'
gem 'slim', '~> 2.1.0'
gem 'thread_safe', '0.3.6'
gem 'tilt', '2.0.7'
else
gem 'nokogiri', '~> 1.6.0'
gem 'slim', '<= 3.0.7'
end
else
gem 'nokogiri', '~> 1.6.0'
gem 'slim', '<= 3.0.7'
end
else
gem 'nokogiri', '~> 1.6.0'
gem 'nokogiri', '~> 1.7.0' if Gem::Platform.local =~ 'x86-mingw32' || Gem::Platform.local =~ 'x64-mingw32'
gem 'racc', '~> 1.4.0' if RUBY_VERSION == '2.1.0' && RUBY_ENGINE == 'rbx'
end
elsif ruby_version < (Gem::Version.new '2.2.0')
gem 'nokogiri', '~> 1.7.0' if Gem::Platform.local =~ 'x86-mingw32' || Gem::Platform.local =~ 'x64-mingw32'
end
gem 'racc', '~> 1.4.0' if RUBY_VERSION == '2.1.0' && RUBY_ENGINE == 'rbx'
end

group :doc do
Expand Down
3 changes: 2 additions & 1 deletion asciidoctor.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ Gem::Specification.new do |s|
s.add_development_dependency 'rspec-expectations', '~> 2.14.0'
# slim is needed for testing custom templates
s.add_development_dependency 'slim', '~> 3.0.0'
s.add_development_dependency 'thread_safe', '~> 0.3.0'
# concurrent-ruby is defined in Gemfile due to enforcement of minimum required Ruby version
#s.add_development_dependency 'concurrent-ruby', '~> 1.0.0'
# tilt is needed for testing custom templates
s.add_development_dependency 'tilt', '~> 2.0.0'
s.add_development_dependency 'minitest', '~> 5.3.0'
Expand Down
18 changes: 10 additions & 8 deletions lib/asciidoctor/converter/factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module Converter
# DocBook 5} and {DocBook45Converter DocBook 4.5}, as well as any custom
# converters that have been discovered or explicitly registered.
#
# If the {https://rubygems.org/gems/thread_safe thread_safe} gem is
# If the {https://rubygems.org/gems/concurrent-ruby concurrent-ruby} gem is
# installed, access to the default factory is guaranteed to be thread safe.
# Otherwise, a warning is issued to the user.
class Factory
Expand All @@ -32,8 +32,8 @@ class << self

# Public: Retrieves a singleton instance of {Factory Converter::Factory}.
#
# If the thread_safe gem is installed, the registry of converters is
# initialized as a ThreadSafe::Cache. Otherwise, a warning is issued and
# If the concurrent-ruby gem is installed, the registry of converters is
# initialized as a Concurrent::Hash. Otherwise, a warning is issued and
# the registry of converters is initialized using a normal Hash.
#
# initialize_singleton - A Boolean to indicate whether the singleton should
Expand All @@ -44,14 +44,16 @@ class << self
# Returns the default [Factory] singleton instance
def default initialize_singleton = true
return @__default__ || new unless initialize_singleton
# FIXME this assignment is not thread_safe, may need to use a ::Threadsafe helper here
# FIXME this assignment itself may not be thread safe; may need to use a helper here
@__default__ ||= begin
# NOTE .to_s hides require from Opal
require 'thread_safe'.to_s unless defined? ::ThreadSafe
new ::ThreadSafe::Cache.new
unless defined? ::Concurrent::Hash
# NOTE .to_s hides require from Opal
require ::RUBY_MIN_VERSION_1_9 ? 'concurrent/hash'.to_s : 'asciidoctor/core_ext/1.8.7/concurrent/hash'.to_s
end
new ::Concurrent::Hash.new
rescue ::LoadError
include Logging unless include? Logging
logger.warn 'gem \'thread_safe\' is not installed. This gem is recommended when registering custom converters.'
logger.warn 'gem \'concurrent-ruby\' is not installed. This gem is recommended when registering custom converters.'
new
end
end
Expand Down
14 changes: 7 additions & 7 deletions lib/asciidoctor/converter/template.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
autoload :ThreadSafe, 'thread_safe'

# encoding: UTF-8
module Asciidoctor
# A {Converter} implementation that uses templates composed in template
Expand All @@ -22,8 +20,8 @@ module Asciidoctor
# backend format (e.g., "html5").
#
# As an optimization, scan results and templates are cached for the lifetime
# of the Ruby process. If the {https://rubygems.org/gems/thread_safe
# thread_safe} gem is installed, these caches are guaranteed to be thread
# of the Ruby process. If the {https://rubygems.org/gems/concurrent-ruby
# concurrent-ruby} gem is installed, these caches are guaranteed to be thread
# safe. If this gem is not present, there is no such guarantee and a warning
# will be issued.
class Converter::TemplateConverter < Converter::Base
Expand All @@ -36,8 +34,10 @@ class Converter::TemplateConverter < Converter::Base
}

begin
# triggers autoload of thread_safe
@caches = { :scans => ::ThreadSafe::Cache.new, :templates => ::ThreadSafe::Cache.new }
unless defined? ::Concurrent::Hash
require ::RUBY_MIN_VERSION_1_9 ? 'concurrent/hash' : 'asciidoctor/core_ext/1.8.7/concurrent/hash'
end
@caches = { :scans => ::Concurrent::Hash.new, :templates => ::Concurrent::Hash.new }
rescue ::LoadError
@caches = { :scans => {}, :templates => {} }
end
Expand Down Expand Up @@ -76,7 +76,7 @@ def initialize backend, template_dirs, opts = {}
end
case opts[:template_cache]
when true
logger.warn 'gem \'thread_safe\' is not installed. This gem is recommended when using the built-in template cache.' unless defined? ::ThreadSafe::Cache
logger.warn 'gem \'concurrent-ruby\' is not installed. This gem is recommended when using the built-in template cache.' unless defined? ::Concurrent::Hash
@caches = self.class.caches
when ::Hash
@caches = opts[:template_cache]
Expand Down
5 changes: 5 additions & 0 deletions lib/asciidoctor/core_ext/1.8.7/concurrent/hash.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require 'thread_safe'

module Concurrent
Hash = ::ThreadSafe::Cache
end
4 changes: 2 additions & 2 deletions test/converter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@
doc = Asciidoctor::Document.new [], :template_dir => template_dir
doc.converter
caches = Asciidoctor::Converter::TemplateConverter.caches
if defined? ::ThreadSafe::Cache
assert_kind_of ::ThreadSafe::Cache, caches[:templates]
if defined? ::Concurrent::Hash
assert_kind_of ::Concurrent::Hash, caches[:templates]
refute_empty caches[:templates]
paragraph_template_before = caches[:templates].values.find {|t| File.basename(t.file) == 'block_paragraph.html.haml' }
refute_nil paragraph_template_before
Expand Down