search

juusaw / amp-jekyll

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

loop over each page in a thread to speed up generation #26

Closed pauldambra closed 2 years ago

pauldambra commented 7 years ago

PR referencing #25

(untested here but only because I must have done something silly while trying to sort out the kids breakfast and check my work :))

juusaw commented 7 years ago

One thing that I fear is what happens when spawning a new thread for each post on very large sites (thousands of posts). I think there's a chance that the system runs out of threads though that would require a very large number of posts (my OS seems to set the maximum to 61028 by default).

A more realistic concern might be that with a large number of short tasks the overhead of context switching between threads becomes a bottleneck for performance. I think it would be a good idea to implement a setting to disable multithread usage if a user encounters a problem related to this. I'll open an issue for that.

pauldambra commented 7 years ago

Hmm, yeah, that's a good point. Might be worth pushing​with into a queue and having a configurable number of threads to pull and then process the pages. Could be getting a bit fancy at that point though :)

On Sat, 1 Apr 2017, 16:00 Juuso Mikkonen, notifications@github.com wrote:

One thing that I fear is what happens when spawning a new thread for each post affects very large sites (thousands of posts). I think there's a chance that the system runs out of threads though that would require a very large number of posts (my OS seems to set the maximum to 61028 by default).

A more realistic concern might be that with a large number of short tasks the overhead of context switching between threads becomes a bottleneck for performance. I think it would be a good idea to implement a setting to disable multithread usage if a user encounters a problem related to this. I'll open an issue for that.

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

pauldambra commented 7 years ago

@juusaw Added an implementation that limits the number of threads. I've pulled the number of threads from an environment variable to try and keep things simple.

pauldambra commented 3 years ago

Ha, I've just seen this is open. Feel free to merge it or close it :)