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

Resize images asynchronously #171

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gerdint
Copy link
Contributor

@gerdint gerdint commented Oct 31, 2016

Yields a 2x speed-up on my dual-core i5 machine.

However, as it stands each size of an image to be resized yields a new subprocess potentially spawning many subprocesses concurrently if the site contains a lot of images (such as my own). This increases memory usage and may therefore cause disk swapping if memory is limited. Does not seem to be a big issue on my 8GB SSD-equipped machine but I understand performance characteristics may be different on other machines.

It may perhaps be preferable to a have a job server making sure that no more than N processes are spawned concurrently. A useful but potentially conservative value for N may be the number of processor cores.

OR perhaps I'm over-engineering here. WDYT?

;; detect that work was finished prematurely and restart everything somehow.
(let-values ([(proc in out err) (apply subprocess
(current-output-port)
(current-input-port)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You you may notice that I never explicitly close these ports, instead relying on the OS the close them once subprocess is finished. I think this should be OK.

@gerdint
Copy link
Contributor Author

gerdint commented Nov 9, 2016

I went ahead and implemented a simple process pool. Really helped to lower memory usage while retaining the faster build times (on multi-core machines).

This was my first foray into concurrent Racket programming. As usual Racket seems to excel in this area and while things felt intuitive and I tried to keep things simple you may want to review the last commit more carefully.

Personally I feel pretty happy with the state of things now and barring any bugs I do not see any need for more work on this feature. The srcset-async branch is only two commits on top of the srcset one so once the latter is squashed it should only be a matter of a trivial rebase onto that for merging it.

Add the srcset and sizes attribute to img tags whose class is
'img-responsive' or whose parent element class is 'figure' (default
Markdown behavior). Each image is made avaiable in three
version (roughly intended to match a mobile, 1x desktop and 2x/hi-dpi
desktop clients). Images are cached in a configurable directory.

In practice the above means that images in Markdown posts are made
responsive by default, and for Scribble posts the "img-responsive" class
can be added like so:

@image["img/some-image.gif" #:style "img-responsive"]

See example/.frogrc for documentation on added parameters. There are
also two additional parameters controlling resized image sizes and the
default size, see bottom of frog/params.rkt.

Depends on ImageMagick® for size identification and scaling.
Defaults to 1.5*<number-of-cores> concurrent processes
@gerdint
Copy link
Contributor Author

gerdint commented Jan 11, 2017

I have spotted one issue when using this together with the Frog watch mode. Will look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant