Make Chrome mlock itself |
||
Issue descriptionI'm told we have hacks in the kernel that try to keep us from swapping Chrome out. I think the consensus is that these are undesirable, and that we should teach Chrome how to mlock itself instead.
,
Dec 10
CL to lift ulimits in `ui` (which spawns session_manager, which spawns Chrome): https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1370846 As noted, it's not really tested so far. Not poking a reviewer until I've actually tested it with an actual patch to Chrome/etc.
,
Dec 10
i'm not sure the ui job is the right place for this as that affects not just the browser processes but the entire process tree from session manager down. you probably want to do something like we do for fds: https://chromium-review.googlesource.com/448356 and leave it to the browser to pick sane soft limits for itself on a per-process basis.
,
Dec 11
,
Dec 11
Yeah I made a few prototype CLs to do this -- I'll put them up
,
Dec 11
https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1370851 I got a strange error while uploading that CL: Errors in PROJECT *chromiumos/platform2*! COMMIT 9f088818: Description: >libchromeos-ui: increase limit on mlockable memory > >This increases the limit on mlockable memory so that Chrome may lock >text pages that can be backed by huge pages. This should help prevent >the loss of the huge pages when we get into memory pressure >situations. > >BUG=chromium:913700 >TEST=build chrome with mlock patches and see that chrome was able to >lock text > >Change-Id: Iaee105d1a85b3f99ee13714e4602d14cdcd63856 > > Errors: * clang-format.py errors/warnings /mnt/host/source/chromium/src/buildtools/linux64/clang-format: /lib64/libtinfo.so.5: no version information available (required by /mnt/host/source/chromium/src/buildtools/linux64/clang-format) /mnt/host/source/chromium/src/buildtools/linux64/clang-format: /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.x/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by /mnt/host/source/chromium/src/buildtools/linux64/clang-format) error: `/mnt/host/source/chromium/src/buildtools/linux64/clang-format -style=file -lines=227:233 -assume-filename=libchromeos-ui/chromeos/ui/chromium_command_builder.cc` failed clang-format failed: return code: 2; command: /mnt/host/source/chromium/src/buildtools/clang_format/script/git-clang-format --binary /mnt/host/source/chromium/src/buildtools/linux64/clang-format --diff --style file '9f08881867525b95c789b1e0e0f7d3aa8f236fad^' 9f08881867525b95c789b1e0e0f7d3aa8f236fad -- arc/ authpolicy/ biod/ bluetooth/ buffet/ cfm-device-updater/ crash-reporter/ cros-disks/ crosdns/ diagnostics/ dlcservice/ goldfishd/ imageloader/ installer/ libchromeos-ui/ libcontainer/ libpasswordprovider/ login_manager/ metrics/ mist/ modemfwd/ oobe_config/ portier/ power_manager/ run_oci/ smbprovider/ vm_tools/ vpn-manager/ wimax_manager/ cmd=['/mnt/host/source/chromium/src/buildtools/clang_format/script/git-clang-format', '--binary', '/mnt/host/source/chromium/src/buildtools/linux64/clang-format', '--diff', '--style', 'file', '9f08881867525b95c789b1e0e0f7d3aa8f236fad^', '9f08881867525b95c789b1e0e0f7d3aa8f236fad', '--', 'arc/', 'authpolicy/', 'biod/', 'bluetooth/', 'buffet/', 'cfm-device-updater/', 'crash-reporter/', 'cros-disks/', 'crosdns/', 'diagnostics/', 'dlcservice/', 'goldfishd/', 'imageloader/', 'installer/', 'libchromeos-ui/', 'libcontainer/', 'libpasswordprovider/', 'login_manager/', 'metrics/', 'mist/', 'modemfwd/', 'oobe_config/', 'portier/', 'power_manager/', 'run_oci/', 'smbprovider/', 'vm_tools/', 'vpn-manager/', 'wimax_manager/'] Please report this to the clang team.
,
Dec 11
Thanks for the patches! Did we want to only mlock hugepages, or did we want to mlock everything in the binary? I was thinking that HP-only made a lot of sense <4.4, but mlock2+MLOCK_ONFAULT looked pretty attractive for everything that supports it. I ask because in the 'mlock all the things' case, we'd probably want to bump the 64MB to 256MB, or some other suitably > sizeof(chrome's .text) number.
,
Dec 11
I was only locking what's in the huge pages. I saw the mlock2 + MLOCK_ONFAULT stuff but I'm not sure if it's necessary, so I was being conservative here. I'd want to avoid locking things in that say only get used on startup or at certain times, because that would just be a waste. The chrome patch is here: https://chromium-review.googlesource.com/c/chromium/src/+/1371206
,
Dec 11
> I got a strange error while uploading that CL: make sure you sync your entire tree so you get a newer clang-format. or run repo upload outside of the sdk.
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07f4d1a58bc25e0c57abee9441c11e847b1c0683 commit 07f4d1a58bc25e0c57abee9441c11e847b1c0683 Author: Sonny Rao <sonnyrao@chromium.org> Date: Tue Dec 11 23:37:40 2018 chromeos_hugepage: mlock in the area covered by huge pages We are putting our most used text into a region that can be backed by huge pages, however we can lose those pages when we encounter memory pressure, so we should be locking them into place to avoid that. R=llozano@chromium.org Bug: 913700 Change-Id: If9ae32ae046a21d5bf7af4e43624323b3429d9f8 Reviewed-on: https://chromium-review.googlesource.com/c/1371206 Reviewed-by: George Burgess <gbiv@chromium.org> Commit-Queue: Sonny Rao <sonnyrao@chromium.org> Cr-Commit-Position: refs/heads/master@{#615724} [modify] https://crrev.com/07f4d1a58bc25e0c57abee9441c11e847b1c0683/chromeos/hugepage_text/hugepage_text.cc
,
Dec 12
One other thing I'm noticing is that it seems like we don't have huge page text on the browser process, does that seem correct? I was testing with some of these changes and I noticed the huge page setup is happening in the zygote, which is spawning all of the renderers and some other processes, but if I look in /proc/<pid>/smaps for the browser process, I'm not seeing any usage of huge pages. I think to remove our kernel hack with the file pages, we'll need to do something for the browser process as well.
,
Dec 12
To give some context, most of the UI including handling of input events is happening in the browser process, so if we lose text pages there, the user generally notices janky UI and cursor. After chatting with Luis, it sounds like it's possible to just to mlock in the same code pages in browser process without any loader changes. George, do you know how to do that?
,
Dec 12
yes, by design huge pages were only enabled on the renderer process. That is where we saw a perf improvement. We could just do the mlock trick on the browser, without huge pages.
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/c4851b7d352f32a0ad84b9ca6eb5e32b7171486d commit c4851b7d352f32a0ad84b9ca6eb5e32b7171486d Author: Sonny Rao <sonnyrao@chromium.org> Date: Thu Dec 13 01:04:54 2018 libchromeos-ui: increase limit on mlockable memory This increases the limit on mlockable memory so that Chrome may lock text pages that can be backed by huge pages. This should help prevent the loss of the huge pages when we get into memory pressure situations. BUG=chromium:913700 TEST=build chrome with mlock patches and see that chrome was able to lock text Change-Id: Iaee105d1a85b3f99ee13714e4602d14cdcd63856 Reviewed-on: https://chromium-review.googlesource.com/1370851 Commit-Ready: Sonny Rao <sonnyrao@chromium.org> Tested-by: Sonny Rao <sonnyrao@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/c4851b7d352f32a0ad84b9ca6eb5e32b7171486d/libchromeos-ui/chromeos/ui/chromium_command_builder.cc
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5e93420c2ed448d09b16cbb6c4800d825d1d7a96 commit 5e93420c2ed448d09b16cbb6c4800d825d1d7a96 Author: George Burgess IV <gbiv@chromium.org> Date: Thu Dec 13 20:56:53 2018 hugepage_text: note that we now mlock things CL 1371206 made us mlock hugepages. I think this deserves a callout in docs, and tweaks to function names so they more accurately describe what we're doing. This CL does all of that. No functional change is intended. Bug: 913700 Test: Chrome still builds for falco, which runs this code. Change-Id: If8dfe93e147bec47c100bb2eb58ba05625c9bcfd Reviewed-on: https://chromium-review.googlesource.com/c/1374960 Reviewed-by: Ken Rockot <rockot@google.com> Reviewed-by: George Burgess <gbiv@chromium.org> Commit-Queue: George Burgess <gbiv@chromium.org> Cr-Commit-Position: refs/heads/master@{#616425} [modify] https://crrev.com/5e93420c2ed448d09b16cbb6c4800d825d1d7a96/chrome/app/chrome_main_delegate.cc [modify] https://crrev.com/5e93420c2ed448d09b16cbb6c4800d825d1d7a96/chromeos/hugepage_text/hugepage_text.cc [modify] https://crrev.com/5e93420c2ed448d09b16cbb6c4800d825d1d7a96/chromeos/hugepage_text/hugepage_text.h
,
Dec 23
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/bca375e4615ac617d7f652d9bc802c300f38a8bd commit bca375e4615ac617d7f652d9bc802c300f38a8bd Author: George Burgess IV <gbiv@chromium.org> Date: Sun Dec 23 22:46:12 2018 tast-tests: Be sure that we're mlocking memory in Chrome For consistent performance under memory pressure, we try to mlock pieces of Chrome. This tast ensures that this mlocking still happens in the future, since the only indication we otherwise have of it disappearing is a degraded UX and a few LOG(INFO) messages. BUG=chromium:913700 TEST=tast run $(cat /tmp/cros_ip) platform.ChromeMlocked Change-Id: I5bc90effc49e562cbf7e1770238cc10afa2ba5c6 Reviewed-on: https://chromium-review.googlesource.com/1381891 Commit-Ready: George Burgess <gbiv@chromium.org> Tested-by: George Burgess <gbiv@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [add] https://crrev.com/bca375e4615ac617d7f652d9bc802c300f38a8bd/src/chromiumos/tast/local/bundles/cros/platform/chrome_mlocked.go |
||
►
Sign in to add a comment |
||
Comment 1 by g...@chromium.org
, Dec 10