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

Limit the processes which are created #242

Open
FibreFoX opened this issue Feb 19, 2016 · 15 comments
Open

Limit the processes which are created #242

FibreFoX opened this issue Feb 19, 2016 · 15 comments

Comments

@FibreFoX
Copy link

Hi,

I'm using this tool as part of our development-pipeline running gulp. We are using pageres as part of our daily development on our local machines when developing static html-websites. There exists a task where we can create screenshots with given resolutions for all of our created html-files.

Is it possible to reduce the processes which are spawn to have a given limit?

It is already mentioned, that the mutiple processes are concurrent, but having a lot of resolutions (at least 10 sometimes for each page) creates a big memory- and process-increase which slows down the development-system by making it less responsive while phantomjs-processes are created/executed.

As workaround we were creating a new instance of pageres inside the .then() via callback, often resulting in warnings about "SIGINT listeners above 11" which seems not to work on our continuous-integration server.

Is there a nice way to take a screenshot of a lot websites (at least 10) with a lot of resolutions (at least 5 to 10) without having all processes created at the same time? How to create a nice queue for this?

@SamVerschueren
Copy link
Contributor

Would something like this be a solution?

const gulp = require('gulp');
const Pageres = require('pageres');

const pages = ['http://yeoman.io', 'http://todomvc.org'];

gulp.task('screenshots', () => {
     return pages.reduce((promise, page) => {
          return promise.then(() => {
               return new Pageres()
                   .src(page, ['1280x1024', '1920x1080', '480x320', '1024x768', 'iphone 5s']);
                   .dest('results')
                   .run();
          });
     }, Promise.resolve());
});

It creates a promise chain that will wait until the previous webpage is processed. It will take a little longer but in this case only creates 5 concurrent processes. If event that is too much, you can split up the promise chain into event smaller parts. One or two resolutions at a time.

@FibreFoX
Copy link
Author

Thanks a lot, I'll try that and will report back.

@SamVerschueren
Copy link
Contributor

And off course you can log the progress every time you finished a website. Could be helpfull if it takes quite some time.

@FibreFoX
Copy link
Author

Thanks a lot, that kinda worked for me. My solution right now:

    function screenshotter(webserverAddress, targetScreenshotFolder, cb){
        // https://github.com/sindresorhus/pageres/issues/242
        console.log("Starting to take screenshots for", existingWebpageFilenames.length, "pages with resolutions", configuration.resolutions);

        // push this for having callback-functionality
        existingWebpageFilenames.push(cb);

        existingWebpageFilenames.reduce(function (promise, page){
            if(typeof page === "function"){
                promise.then(function(){
                    page();
                });
            }
            return promise.then(function(){
                console.log("Making screenshots of page", page);
                var screenshotDestination = targetScreenshotFolder + "/" + page.replace(".html","");
                var pageres = new Pageres({
                    delay: 2,
                    crop:true,
                    filename:page + "-<%= size %>"
                });
                return pageres
                    .src(webserverAddress + page, configuration.resolutions)
                    .dest(screenshotDestination)
                    .run()
                ;
            });
        }, Promise.resolve());
    }

I had to push some callback into the array, because we are having some cleanup-tasks after the screenshots were taken. In the future we are planing to make regression-test with that approach.

Although this solves some problem for me, it doesn't make it possible to set a limit of processes, might to be documented somewhere. The first time I experimente with Pageres (very cool project by the way 😸 ), I wasn't expecting that behaviour.

Feel free to close this issue, if you don't feel this as being some problem for you.

@SamVerschueren
Copy link
Contributor

You are using gulp to take the screenshots right? Why not do it like this then?

gulp.task('screenshotter', () => {
    // screenshot code here

   return screenshotter(); // -> returns the promise
});

gulp.task('cleanup', ['screenshotter'], () => {
    // This will wait untill screenshotter finishes

    // Do the cleanup stuff
});

I don't think limiting the processes is something pageres should do. It would be possible though to create a module that depends on pageres and creates the promise chain for you depending on the number of processes you want. It's a little bit tricky, but if well tested it should definitely work.

const screenshotter = require('screenshotter');

const webpages = {
     'http://yeoman.io': ['1280x1024', '1920x1080', '480x320', '1024x768', 'iphone 5s'],
     'http://todomvc.org': ['1920x1080', '1024x768', 'iphone 5s', 'iphone 6']
};

screenshotter(webpages, {processLimit: 1}); //-> Call pageres with one resolution at a time

screenshotter(webpages, {processLimit: 2}); //-> Call pageres with two resolutions at a time

Dividing the above examples per two is harder then one off course. Because the 3rd time, you would have to take a screenshot of yeoman.io for the iphone 5s and from todomvc.org with a resolution of 1920x1080. Promise.all to the rescue in this case.

I let @sindresorhus and @kevva decide if it would be a useful addition.

@SamVerschueren
Copy link
Contributor

@FibreFoX If you really have to use the cb, the function can be written cleaner :).

    function screenshotter(webserverAddress, targetScreenshotFolder, cb){
        // https://github.com/sindresorhus/pageres/issues/242
        console.log("Starting to take screenshots for", existingWebpageFilenames.length, "pages with resolutions", configuration.resolutions);

        existingWebpageFilenames.reduce(function (promise, page){
            return promise.then(function(){
                console.log("Making screenshots of page", page);
                var screenshotDestination = targetScreenshotFolder + "/" + page.replace(".html","");
                var pageres = new Pageres({
                    delay: 2,
                    crop:true,
                    filename:page + "-<%= size %>"
                });
                return pageres
                    .src(webserverAddress + page, configuration.resolutions)
                    .dest(screenshotDestination)
                    .run()
                ;
            });
        }, Promise.resolve()).then(function () {
            cb();
        });
    }

@FibreFoX
Copy link
Author

@SamVerschueren thanks for the cleaner solution ;) I'll use that.

I've already using that inside a module :D I was just writing an excerpt here.

@FibreFoX
Copy link
Author

@SamVerschueren Just a short question, which might be related to #221

I'm running the provided solution here, which works nice, but in the case there is some javascript-exception on the page I'm visiting, the forked PhantomJS-processes are still there, is there any way to kill that still-running processes? (running on windows 8.1) Is there any option I can set to force-shutdown that PhantomJS-instance?

@SamVerschueren
Copy link
Contributor

@FibreFoX There is no option for this. Not sure why they aren't being shut down. Maybe @sindresorhus or @kevva could provide an answer?

@sindresorhus
Copy link
Owner

JavaScript errors are common on websites in the wild. It doesn't warrant aborting the screenshot capturing.

@FibreFoX
Copy link
Author

@sindresorhus The generated PhantomJS-processes are still living with the provided solution, so I can't follow you what to do, because Pageres doesn't give me any option to force-shutdown the still-living processes.

@sindresorhus
Copy link
Owner

The processes should be shutdown by Pageres when done. If it's not doing that it might be a bug. Don't really have time to look into this.

@FibreFoX
Copy link
Author

To help debugging, I'll try to create something locally reproducable. Thanks so far.

@SamVerschueren
Copy link
Contributor

@FibreFoX You can use promise-concurrent if you want to limit the processes. It won't help you with the process issue though.

@victorferraz
Copy link
Contributor

@FibreFoX hey man, you can use async module (https://github.com/caolan/async), with the method eachLimit, you can set the numbers of process in the same time,

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

No branches or pull requests

4 participants