search

juusaw / amp-jekyll

Build Accelerated Mobile Page versions of your Jekyll posts
MIT License
281 stars 59 forks source link

Speeding up generation #25

Open pauldambra opened 7 years ago

pauldambra commented 7 years ago

Thanks for this plugin!

I found builds were taking 18 seconds for me.

require 'thread'
require 'thwait'

  # Generates a new AMP post for each existing post
  class AmpGenerator < Generator
    priority :low
    def generate(site)
      dir = site.config['ampdir'] || 'amp'
      threads = site.posts.docs.map do |post|
        Thread.new do
          index = AmpPost.new(site, site.source, File.join(dir, post.id), post)
          index.render(site.layouts, site.site_payload)
          index.write(site.dest)
          site.pages << index
        end
      end
      ThreadsWait.all_waits(*threads)
    end
  end
end

I amended the main loop to start a new thread for each AmpPost to be generated and then wait for them all to finish.

That took my build from 18 seconds down to 7 seconds.

Happy to open a PR if you like?

juusaw commented 7 years ago

Sounds good! I'd be happy to test the parallelization and accept the PR.

pauldambra commented 7 years ago

Cool. Might be the weekend before I get to take a proper look...

On Tue, 28 Mar 2017, 08:05 Juuso Mikkonen, notifications@github.com wrote:

Sounds good! I'd be happy to test the parallelization and accept the PR.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/juusaw/amp-jekyll/issues/25#issuecomment-289682255, or mute the thread https://github.com/notifications/unsubscribe-auth/AA8G8UDQiPV60bnEduqUukThFPVo8ciFks5rqLEmgaJpZM4Ml9bN .

pauldambra commented 7 years ago

Just looking at adding this and having to include changes made here since I copied the files for my blog...

One of the changes added a layout:amp to the top of the AMP layout

---
layout: amp
---
<!doctype html>
<html amp lang="en">

but that just causes warnings and doesn't render anything for me...

Build Warning: Layout 'amp' requested in amp/2016/yarn/index.html does not exist.

I think I must be misunderstanding something..?

juusaw commented 7 years ago

Hi, did you update all the changed files before applying your updates? I managed to get the threaded version working from the latest version of master.

class AmpGenerator < Generator
    priority :low
    def generate(site)
      dir = site.config['ampdir'] || 'amp'
      threads = site.posts.docs.map do |post|
        next if post.data['skip_amp'] == true
        Thread.new do
          site.pages << AmpPost.new(site, site.source, File.join(dir, post.id), post)
        end
      end
      ThreadsWait.all_waits(*threads)
    end
  end
pauldambra commented 7 years ago

:) It's worked for me without the skip for many builds now. If it's working for you too then was doing too many things at once this morning... I must have done something silly