empty result on multiple parallel calls

[bezoerb]

Hey @pocketjoso,

i've just stumbled across an error where penthouse returns an empty string when it's called multiple times in parallel.
Testcase: https://github.com/bezoerb/penthouse/blob/multipleFails/test/basic-tests.js#L91
Result: https://travis-ci.org/bezoerb/penthouse/jobs/58228239

Let me know if i can help fixing this.

[pocketjoso]

Hi, thanks for reporting the issue. I'm a bit busy at the moment, hoping to be able to look into it no later than next weekend.

My first thought is that it's probably an upstream problem, with phantomjs. What phantomjs version are you running, 1.9.8? Does using 2.x.x make any difference? That would be my first test..

[bezoerb]

I'll give it a try
Thanks

[fatso83]

@bezoerb How did that turn out for you; did you get to try out a later version?

[bezoerb]

i could not make penthouse play nicely together with phantomjs 2.0.0 installed via homebrew but i could fix the issue in grunt-critical by making the calls in series rather than parallel. It seems to me that this is some sort of multithreading issue. Some times my results got mixed up so i didn't receive the same results for a file in a set of 20-30 different files during multiple runs compared to multiple runs with just that one file.

[fatso83]

That is quite interesting, and if you find a way to reproduce that behavior I would like to take a look at it. Can think of a few reasons that might cause this, but need a reproducable test case.

[bezoerb]

See https://travis-ci.org/addyosmani/critical/jobs/59405654 and https://travis-ci.org/bezoerb/penthouse/jobs/58228239
In that case, some of the parallel calls return an empty result.
@fatso83 what about the testcase mentioned above?
If you could give some hints where to look for the "few reasons" i would take a look and see if i can do a PR

[bezoerb]

@pocketjoso are there any updates on this?

[pocketjoso]

Hmm. Looking at your test; are you expecting to get a full css response on each iteration? I don't think you can rely on getting that, when running that many calls in parallel. Penthouse just fires up one new PhantomJS instance every time it's called, meaning you will end up with lots of browser starting and running. My guess would be the empty strings are from timeouts or just phantomjs crashing.

In order to support multiple parallel penthouse calls I think we need a solution that re-uses the same instance of phantomjs. One way to do this is to run the instance on a local server (but that would have to be a separate repo). Another possible solution which @fatso83 suggested before is phantomjs clusters.

So unless I'm mistaken what you're trying to do is not possible out of the box with penthouse (or phantomjs). It requires some new code to be written.

[bezoerb]

@pocketjoso thanks for the quick reply. I tried to simulate the behavior when running critical via gulp on multiple files. Any ideas how this could be solved on our side?

[bezoerb]

seems like critical runs in series when using with gulp.

[eimfach]

@bezoerb I had that problem with your critical module too. The Problem was that there were running too many instances communicating via a single Socket with the phantomjs process. II then spawned new childprocesses for each instance and with a max amount x at a time. And, take a look at my PR and pls provide some feedback since the critical module does not handle the delegated process handling from the penthouse module, which could introduce errors

[bezoerb]

closing this at this could not be resolved in penthouse.