Reuse of electron.BrowserWindow / BrowserWindow pool

[749]

I am planning to run a site which may get spikes of serveral hundreds of PDF generation requests.
Of course this cannot be handled by a single server, so I have put your library into a self-contained docker.

However my local tests show that most of the time seems to be wasted on trying to open the BrowserWindow and then closing it again.
Would it be possible to reuse a pool of a configurable number of open BrowserWindow instances?

[codecounselor]

That seems like a reasonable idea. Feel free to submit a PR.

What exactly are you running inside of your docker container? You're keeping the Electron process running right?

I haven't profiled my export server to see where the hotspots are, and we don't service hundreds of requests per second so this hasn't been a problem for me yet.

[749]

My Plan was to run an node express with the electron-pdf integrated handling the requests.
During a local test I noticed that the number of opened BrowserWindow instances equals the number of requests.

Although this is not an issue with only a couple of requests, not having a limitation on the number of open BrowserWindows means a simple DoS of a couple 100 requests will quickly fill up the memory on the VM/server.

So the solution IMHO is to make sure the requests are queued and the number of "worker" BrowserWindow instances can not exceed a configurable number. I think that it would gain a couple of milliseconds per request to keep the BrowserWindow open and to reuse it.

[codecounselor]

As long as all of the browser windows shared the same options that would be fine.

I won't have time to implement this feature myself anytime soon. But if you want to submit a PR I'm fine adding that functionality to the project.

A few thoughts:

1. To maintain backwards compatibility this should not be the default behavior.
2. I suggest adding a new environment variable process.env.ELECTRONPDF_BROWSER_POOL_SIZE with a default value of zero indicating no pooling.
3. If you're concerned bout DoS you probably also want a process.env.ELECTRONPDF_REQUEST_QUEUE_MAX_SIZE so requests over a reasonable threshold (e.g. 100) just gets dropped instead of being added to your in-memory queue. You can figure out what that number should be based on your average response time and connection timeout settings.

[codecounselor]

@749 Is this still something you're interested in? I am starting to work on this for my own use case, but I suspect it is different than yours.

I'll mention a few points about that here.

1. For requests that access a url that has authentication involved, it is necessary that the partitionId be set. That is done here https://github.com/fraserxu/electron-pdf/blob/master/lib/exportJob.js#L335. Using a pool for all windows is problematic in that I don't see a way to set this partition apart from the webPreferences object that has to be passed in on the BrowserWindow constructor.
2. Any additional web preferences that a client passes via the browserConfig have this same limitation.
3. Because of this, I think it may only really make sense to use a connection pool for requests that contain 2 or more urls. Currently, there is no support for applying different options to multiple urls in single request. So I think this would provide the most benefit to all use cases.

The problem I am about to address is improving the response time for a request with a large number of URLs. By implementing both a pool and support for making the requests concurrently.