Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(59)

Issue 2957693002: Merged changes to avoid OOM for large wasm modules. (Closed)

Created:
3 years, 6 months ago by Mircea Trofin
Modified:
3 years, 5 months ago
Reviewers:
bradnelson
CC:
v8-reviews_googlegroups.com, v8-merges_googlegroups.com
Target Ref:
refs/branch-heads/6.0
Project:
v8
Visibility:
Public.

Description

Merged changes to avoid OOM for large wasm modules. Between M60 and 61, some files got refactored, specifically, module-compiler.* was added to contain parts of wasm-module.*. To back-merge, changes that were made in module-compiler were manually ported to wasm-module. Summary: b29bfffdf9d3c393b98486504ddd7a073a2e0280 045c40d09c9320984bd57d39d68b174099697d1d (fixes async compile for the above) 700901518141609e7096462aabb536de3fc7c438 bug: chromium:732010 [wasm] Throttle the amount of unfinished work to avoid OOM (non-blocking) It is possible that the foreground task is unable to clear the scheduled unfinished work, eventually leading to an OOM. We use either code_range on 64 bit, or the capacity of the code space, as a heuristic for how much memory to use for compilation. The change avoids blocking the background threads while we're over the memory threshold. This is to avoid starving the GC. Bug: v8:6492, chromium:732010 Change-Id: Ic2647d9fa71af4f8cdd2149a434b107cbed3a6c3 Reviewed-on: https://chromium-review.googlesource.com/540763 Commit-Queue: Mircea Trofin <mtrofin@chromium.org>; Reviewed-by: Andreas Haas <ahaas@chromium.org>; Cr-Commit-Position: refs/heads/master@{#46029} [wasm] Reopen CEntryStub handle in deferred scope when async compiling. Bug: chromium:734108 Change-Id: I696b104e3b6b9dd71a60c21baa558d4f1fec1dfb Reviewed-on: https://chromium-review.googlesource.com/541624 Commit-Queue: Brad Nelson <bradnelson@chromium.org>; Reviewed-by: Brad Nelson <bradnelson@chromium.org>; Cr-Commit-Position: refs/heads/master@{#46074} [wasm] Initialize parallel jobs with less memory. Avoid constructing zones and large zone objects when initializing WasmCompilationUnit. The main reason we did that is so we can cache the CEntryStub node, which requires a code object, obtainable only on the main thread. We need that value, however, on background threads, which is also where we need the aforementioned large objects. We only need that for the WasmCompilationUnits being currently compiled, which is a number proportional to the number of background threads provided by the embedder. Specifically, one zone is needed only for the duration of the background compilation, while the second zone needs to survive past that, so the compilation results may be committed to the GC heap as Code objects. The problem with these large objects is that the first allocation in a Zone is at minimum 8KB. We used to allocate 2 zones. For modules with 200K functions, that means 3.2GB of memory pre-allocated before any of it is actually needed. This change attaches a Handle to the CEntryStub on the WasmCompilationUnits, and delays zone creation to when needed. The change also adds a way to cache CEntryStubs in a JSGraph from a given Code handle - limited to the scenario needed by wasm (and removable once we get wasm off the GC heap, which subsumes removing this dependency on CEntryStubs) An additional constraint for this change is that we want it to be easily back-mergeable to address chromium:723899. For the wasm payload in question, collecting the max memory used by d8 using /usr/bin/time --format='(%Xtext+%Ddata %Mmax)', we get the following numbers (in KB): - unchanged: 3307480 - patch 1: 1807140 (45% reduction) - patch 3: 1230320 (62% reduction from first) - patch 5/6: 519368 (84% reduction from first) Bug: chomium:732010, chromium:723899 Change-Id: I45b96792daf8a9c8dc47d45fb52da75945a41401 Reviewed-on: https://chromium-review.googlesource.com/530193 Commit-Queue: Mircea Trofin <mtrofin@chromium.org>; Reviewed-by: Andreas Haas <ahaas@chromium.org>; Cr-Commit-Position: refs/heads/master@{#45880} NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2957693002 Cr-Commit-Position: refs/branch-heads/6.0@{#39} Cr-Branched-From: 97dbf624a5eeffb3a8df36d24cdb2a883137385f-refs/heads/6.0.286@{#1} Cr-Branched-From: 12e6f1cb5cd9616da7b9d4a7655c088778a6d415-refs/heads/master@{#45439} Committed: https://chromium.googlesource.com/v8/v8/+/f9e1999ebce1cf71a273984da31e57e660e38bd5

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+290 lines, -142 lines) Patch
M src/compiler.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/compiler/pipeline.cc View 2 chunks +6 lines, -0 lines 0 comments Download
M src/compiler/wasm-compiler.h View 5 chunks +17 lines, -14 lines 0 comments Download
M src/compiler/wasm-compiler.cc View 25 chunks +87 lines, -72 lines 0 comments Download
M src/compiler/zone-stats.h View 1 chunk +3 lines, -3 lines 0 comments Download
M src/compiler/zone-stats.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M src/wasm/wasm-module.cc View 10 chunks +153 lines, -46 lines 0 comments Download
M test/cctest/wasm/wasm-run-utils.h View 2 chunks +4 lines, -4 lines 0 comments Download
A test/mjsunit/regress/wasm/regression-734108.js View 1 chunk +16 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (8 generated)
Mircea Trofin
3 years, 6 months ago (2017-06-23 22:13:13 UTC) #3
bradnelson
lgtm
3 years, 5 months ago (2017-06-26 20:01:37 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2957693002/1
3 years, 5 months ago (2017-06-26 20:04:13 UTC) #6
commit-bot: I haz the power
CLs for remote refs other than refs/heads/master must contain NOTRY=true and NOPRESUBMIT=true in order for ...
3 years, 5 months ago (2017-06-26 20:04:15 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2957693002/1
3 years, 5 months ago (2017-06-26 20:05:04 UTC) #11
commit-bot: I haz the power
3 years, 5 months ago (2017-06-26 20:05:24 UTC) #14
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/v8/v8/+/f9e1999ebce1cf71a273984da31e57e660e...

Powered by Google App Engine
This is Rietveld 408576698