search

rbuchberger / jekyll_picture_tag

Easy responsive images for Jekyll.
https://rbuchberger.github.io/jekyll_picture_tag/
BSD 3-Clause "New" or "Revised" License
622 stars 106 forks source link

Implement a threadpool for faster builds. #282

Open aebrahim opened 2 years ago

aebrahim commented 2 years ago

Fixes #225

Uses 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.

In my experience on a 4 core/8 thread system, this optimization halves my build times.

aebrahim commented 2 years ago

I don't really know ruby (this is my first ruby PR) so please review carefully!

rbuchberger commented 2 years ago

This is amazing work, thank you! I've been wanting this forever.

I'm out of town on a work trip at the moment so I can't easily pull it down and test it, but I'll get back to you as soon as I can once I get a chance to.

aebrahim commented 2 years ago

Thanks so much! I have 2 versions, this one and #283 - both of which do the same thing but in different ways

rbuchberger commented 2 years ago

@aebrahim - you mind if I go ahead and merge this version, then replace it if we get the global thread pool working?

aebrahim commented 2 years ago

Sounds good to me!

antoinevth commented 1 year ago

Using JPT for an image-heavy website, I was really pleased to see this PR. I've tried to use it, but I came across this error:

/usr/share/gems/gems/concurrent-ruby-1.2.2/lib/concurrent-ruby/concurrent/executor/abstract_executor_service.rb:88:in `block in fallback_action': Concurrent::RejectedExecutionError (Concurrent::RejectedExecutionError)
    from /usr/share/gems/gems/concurrent-ruby-1.2.2/lib/concurrent-ruby/concurrent/executor/ruby_executor_service.rb:27:in `post'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/srcsets/basic.rb:82:in `block in build_files'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/srcsets/basic.rb:81:in `each'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/srcsets/basic.rb:81:in `build_files'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/srcsets/basic.rb:31:in `files'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/srcsets/basic.rb:35:in `to_a'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/srcsets/basic.rb:39:in `to_s'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/output_formats/basic.rb:57:in `add_srcset'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/output_formats/picture.rb:39:in `build_source'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/output_formats/picture.rb:26:in `block in build_sources'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/output_formats/picture.rb:26:in `collect'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/output_formats/picture.rb:26:in `build_sources'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/output_formats/picture.rb:66:in `base_markup'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag/output_formats/basic.rb:10:in `to_s'
    from /usr/share/gems/bundler/gems/jekyll_picture_tag-4c4b75ff77c7/lib/jekyll_picture_tag.rb:71:in `render'

I've also tried using concurrent-ruby in it's 1.1.10 version with no luck. I don't know how I could fix this issue, as I've never developed in Ruby... @aebrahim, have you used this implementation recently ?

rbuchberger commented 1 year ago

@antoinevth - Sorry it's not in a working state! This has been on the back burner a bit for me, but you're not the first one to mention how useful this would be. I'll try and find time to get it working, though no promises.