Skip to content

Commit

Permalink
Implement a global threadpool for faster builds.
Browse files Browse the repository at this point in the history
Fixes rbuchberger#225

This implements a global Concurrent::ThreadPoolExecutor from
concurrent-ruby to avoid memory blowup, and de-duplicates files before
generation so we no longer need to rely on filesystem consistency to
avoid double-generating images.

This is an alternate to rbuchberger#282 with a litle bit more complexity, but with
the added benefits that all image generation can happen in a single
ThreadPool.
  • Loading branch information
aebrahim committed Sep 22, 2022
1 parent e0325d0 commit df74b1d
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 4 deletions.
2 changes: 2 additions & 0 deletions jekyll_picture_tag.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ Gem::Specification.new do |spec|

# addressable is used to url-encode image filenames.
spec.add_runtime_dependency 'addressable', '~> 2.6'
# Needed for parallel excution of builds.
spec.add_runtime_dependency 'concurrent-ruby', '~> 1.1'
# Jekyll versions older than 4.0 are not supported.
spec.add_runtime_dependency 'jekyll', '~> 4.0'
# MIME types are needed for <source> tags' type= attributes.
Expand Down
6 changes: 5 additions & 1 deletion lib/jekyll_picture_tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require_relative 'jekyll_picture_tag/instructions'
require_relative 'jekyll_picture_tag/output_formats'
require_relative 'jekyll_picture_tag/parsers'
require_relative 'jekyll_picture_tag/pool'
require_relative 'jekyll_picture_tag/router'
require_relative 'jekyll_picture_tag/srcsets'
require_relative 'jekyll_picture_tag/utils'
Expand Down Expand Up @@ -68,7 +69,10 @@ def render(context)

''
else
PictureTag.output_class.new.to_s
PictureTag::Pool.start_pool
result = PictureTag.output_class.new.to_s
PictureTag::Pool.stop_pool
result
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/jekyll_picture_tag/output_formats/basic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def build_fallback_image
width: checked_fallback_width
)

image.generate
PictureTag::Pool.generate(image)

image
end
Expand Down
49 changes: 49 additions & 0 deletions lib/jekyll_picture_tag/pool.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
require 'concurrent-ruby'
require 'set'

module PictureTag
# This is a global concurrent-ruby pool for executing tasks in parallel.
# Pool itself should only be used in a single threaded context, and its
# start method must be called prior to submitting to Pool.pool, and stop
# must be called after.
class Pool
# We use a class variable to store pool to emulate a global singleton.
# rubocop:disable Style/ClassVars
@@_pool = nil
# Because we're processing in parallel, we can't use existince in the
# filestystem to prevent duplicate handling of files without a race
# condition, so we manually track generated files here so we don't attempt
# to generate the same image twice.
@@_seen_files = Set[]

def self.start_pool
@@_pool = Concurrent::ThreadPoolExecutor.new(
min_threads: 1, max_threads: Concurrent.processor_count,
max_queue: 2 * Concurrent.processor_count
)
end

def self.start_test_pool
# Stub in an pool which executes immediately to avoid race conditions
# during unit tests.
@@_pool = Concurrent::ImmediateExecutor.new
@@_seen_files = Set[]
end

def self.generate(generated_image)
return if @@_seen_files.include?(generated_image.name)

@@_seen_files.add(generated_image.name)
@@_pool.post do
generated_image.generate
end
end

def self.stop_pool
@@_pool.shutdown
@@_pool.wait_for_termination
@@_seen_files = Set[]
end
end
# rubocop:enable Style/ClassVars
end
4 changes: 2 additions & 2 deletions lib/jekyll_picture_tag/srcsets/basic.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'mime-types'

module PictureTag
# Handles srcset generation, which also handles file generation.
module Srcsets
Expand Down Expand Up @@ -68,8 +69,7 @@ def build_files

# This triggers GeneratedImage to actually build an image file.
files = checked_targets
files.each(&:generate)

files.each { |file| PictureTag::Pool.generate(file) }
files
end

Expand Down
1 change: 1 addition & 0 deletions test/unit/images/test_generated_image_missing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class GeneratedImageMissingTest < Minitest::Test
# Lifecycle
def setup
PictureTag.stubs(config)
PictureTag::Pool.start_test_pool
File.stubs(:exist?).with(destfile).returns(false)
end

Expand Down
2 changes: 2 additions & 0 deletions test/unit/output_formats/output_format_test_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'test_helper'
require 'jekyll_picture_tag'

# The fact that these stubs are so complicated is probably one of those code
# smell things I keep hearing about.
Expand All @@ -7,6 +8,7 @@ module OutputFormatTestHelper
include TestHelper

def base_stubs
PictureTag::Pool.start_test_pool
PictureTag.stubs(config)
stub_srcsets
stub_generated_image
Expand Down
2 changes: 2 additions & 0 deletions test/unit/srcsets/srcsets_test_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'test_helper'
require 'jekyll_picture_tag'

module SrcsetTestHelper
include TestHelper
Expand All @@ -7,6 +8,7 @@ module SrcsetTestHelper
def setup
[100, 150, 200, 300].each { |i| stub_generated(i, gstub(i)) }

PictureTag::Pool.start_test_pool
PictureTag.stubs(config)
end

Expand Down

0 comments on commit df74b1d

Please sign in to comment.