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

Add --disable-dev-shm-usage to default launch flags #1834

Closed
ebidel opened this issue Jan 17, 2018 · 13 comments

Comments

Projects
None yet
4 participants
@ebidel
Copy link
Contributor

commented Jan 17, 2018

There are number of issues running pptr/headless in env like Docker where resources are limited.

Shall we add --disable-dev-shm-usage to the default launch flags so users run into fewer issue?

@aslushnikov

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2018

For some reason it's not Chrome's default behavior, so I wonder what the implications are.
@ebidel are there any drawbacks with this flag? Does it work with headful?

@ebidel

This comment has been minimized.

Copy link
Contributor Author

commented Jan 17, 2018

I've been wondering the same :\

The flag is only on Linux but I haven't tested headful.

Maybe it's not the default b/c of backward compat?

@ebidel

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2018

@ebidel

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2018

From David:

I think it would depend on how are /dev/shm and /tmp mounted.
If they are both mounted as tmpfs I'm assuming there won't be any difference.
if for some reason /tmp is not mapped as tmpfs (and I think is mapped as tmpfs by default by systemd), chrome shared memory management always maps files into memory when creating an anonymous shared files, so even in that case shouldn't be much difference. I guess you could force telemetry tests with the flag enabled and see how it goes.

As for why not use by default, it was a pushed back by the shared memory team, I guess it makes sense it should be useing /dev/shm for shared memory by default.

Ultimately all this should be moving to use memfd_create, but I don't think that's going to happen any time soon, since it will require refactoring Chrome memory management significantly.

https://bugs.chromium.org/p/chromium/issues/detail?id=736452#c65

@aslushnikov

This comment has been minimized.

Copy link
Contributor

commented Jan 19, 2018

Thanks Eric!

As for why not use by default, it was a pushed back by the shared memory team, I guess it makes sense it should be useing /dev/shm for shared memory by default.

Looks like there are reasons to not use --disable-dev-shm-usage by default, so I'd rather not as well.

@ebidel

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2018

Oh. I read that explanation differently as "it's no big deal if you use it. Everything should be more or less the same."

aslushnikov added a commit to aslushnikov/puppeteer that referenced this issue Apr 11, 2018

aslushnikov added a commit that referenced this issue Apr 11, 2018

@Glennvd

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2018

Any reason why this was suddenly implemented while the general consensus was not doing it? Correct me if I'm wrong but enabling this flag in docker containers will cause chromium to use /tmp instead of /dev/shm. Which means it will use the /tmp folder which is a regular filesystem in docker instead of /dev/shm which is shared RAM.

When optimising puppeteer for high throughput, one of the limits you hit is disk IO. This specific case is easily prevented by running docker with --shm-size="". I personally think it's a weird choice to enable this by default, rather than advising that it exists or that docker should run with the shm-size flag.

@aslushnikov

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2018

When optimising puppeteer for high throughput, one of the limits you hit is disk IO. This specific case is easily prevented by running docker with --shm-size=""

@Glennvd tests I did didn't show any difference with/without the flag. Do I get it right that it hits you? How severe is the impact? We should revert this if it regresses performance.

I'd also appreciate if you can share your setup, e.g. where do you run and how big is the load.

@aslushnikov aslushnikov reopened this Apr 13, 2018

@Glennvd

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2018

Hey Aslushnikov, I'll see when I have time to run a clean benchmark.

My setup consists of puppeteer pods running on Kubernetes. These pods autoscale to an average of 10 concurrent puppeteer requests. With an absolute max of 20, and everything higher is queued, but this only happens on very high spikes while the autoscaler needs to scale. Each puppeteer request consists of an incoming HTTP request, that starts a new chromium browser, executes the script and returns either a rendered html or a screenshot.

If these pods are allocated enough CPU, the next bottleneck will be the disk IO, especially in a docker container. Running using --disable-dev-shm-usage will only impact this bottleneck further. To be honest to mitigate this bottleneck I even went further and moved the chromium profiles and chromium executable to /dev/shm completely. Since there we're multiple browsers opening/closing every second, causing repetitive disk reads and writes.

This approach allowed me to scale for peaks up to 400 concurrent sessions, and if needed probably a lot higher.

Now while --disable-dev-shm-usage is a perfectly valid choice when you're just running small tasks, and performance doesn't matter that much. I ultimately think this is a choice that should be left to the developer itself. Especially since not everyone is running in docker to begin with. Otherwise we might start adding flags like --disable-gpu or --no-sandbox by default as well, since these also cause issues in docker.

My proposal would be to leave these flags out of any default configuration, but add a clear FAQ with docker caveats instead.

@ebidel

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2018

Curious if you tried launching chrome and reusing instances across requests? We don't recommend launching chrome per request for reasons like this. Chrome wasn't meant to be used in this way, but it's up to use to support use cases like this :) If you reconnected to chrome instead, how does the perf compare?

Otherwise we might start adding flags like --disable-gpu or --no-sandbox by default as well

In fact, we do still add --disable-gpu because enough users were having problems across platforms.

@aslushnikov

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2018

Curious if you tried launching chrome and reusing instances across requests? We don't recommend launching chrome per request for reasons like this.

@ebidel This is very suboptimal, but seems to be the only way to do a "fresh load" while #85 is not implemented.

@aj-dev

This comment has been minimized.

Copy link
Contributor

commented May 4, 2018

I guess the tips section has to be updated to reflect the recent change in --disable-dev-shm-usage - https://github.com/GoogleChrome/puppeteer/blob/v1.0.0/docs/troubleshooting.md#tips

@aslushnikov

This comment has been minimized.

Copy link
Contributor

commented May 31, 2018

Hey @Glennvd, getting back to this. Did you have a chance to see if the flag caused any perf regressions for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.