Commit e3d957e3 authored by Ben L. Titzer's avatar Ben L. Titzer Committed by Commit Bot

[wasm] "fix" critical memory pressure notification

Previously, we sent a critical low memory pressure notification when
attempting to commit WASM code when the code manager was near the
limit for total amount of code allocated.
https://chromium-review.googlesource.com/c/v8/v8/+/1073412 "fixed" that,
but it causes OOMs on Windows.

Since we no longer have the isolate on the code manager, and thus cannot
send this notification on commit, send the notification upon the next
module creation.

This is still not optimal, but should fix OOM issues for lots of
small modules on Windows.

BUG=v8:7845
R=mstarzinger@chromium.org
CC=clemensh@chromium.org

Change-Id: I6e20d0c1ee9bc6926a83e0c2fbdc9e9e453588ec
Reviewed-on: https://chromium-review.googlesource.com/1098921
Commit-Queue: Ben Titzer <titzer@chromium.org>
Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53700}
parent 4d867c7a
......@@ -881,6 +881,7 @@ bool WasmCodeManager::Commit(Address start, size_t size) {
TRACE_HEAP("Setting rw permissions for %p:%p\n",
reinterpret_cast<void*>(start),
reinterpret_cast<void*>(start + size));
if (!ret) {
// Highly unlikely.
remaining_uncommitted_code_space_.fetch_add(size);
......@@ -889,18 +890,6 @@ bool WasmCodeManager::Commit(Address start, size_t size) {
return ret;
}
bool WasmCodeManager::WouldGCHelp() const {
// If all we have is one module, or none, no GC would help.
// GC would help if there's some remaining native modules that
// would be collected.
if (active_ <= 1) return false;
// We have an expectation on the largest size a native function
// may have.
constexpr size_t kMaxNativeFunction = 32 * MB;
size_t remaining = remaining_uncommitted_code_space_.load();
return remaining < kMaxNativeFunction;
}
void WasmCodeManager::AssignRanges(Address start, Address end,
NativeModule* native_module) {
lookup_map_.insert(std::make_pair(start, std::make_pair(end, native_module)));
......@@ -949,6 +938,20 @@ std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
Isolate* isolate, size_t memory_estimate, uint32_t num_functions,
uint32_t num_imported_functions, bool can_request_more, ModuleEnv& env) {
// TODO(titzer): we force a critical memory pressure notification
// when the code space is almost exhausted, but only upon the next module
// creation. This is only for one isolate, and it should really do this for
// all isolates, at the point of commit.
constexpr size_t kCriticalThreshold = 32 * 1024 * 1024;
bool force_critical_notification =
(active_ > 1) &&
(remaining_uncommitted_code_space_.load() < kCriticalThreshold);
if (force_critical_notification) {
(reinterpret_cast<v8::Isolate*>(isolate))
->MemoryPressureNotification(MemoryPressureLevel::kCritical);
}
VirtualMemory mem;
// If the code must be contiguous, reserve enough address space up front.
size_t vmem_size = kRequiresCodeRange ? kMaxWasmCodeMemory : memory_estimate;
......
......@@ -230,11 +230,6 @@ class V8_EXPORT_PRIVATE WasmCode final {
// Return a textual description of the kind.
const char* GetWasmCodeKindAsString(WasmCode::Kind);
// Note that we currently need to add code on the main thread, because we may
// trigger a GC if we believe there's a chance the GC would clear up native
// modules. The code is ready for concurrency otherwise, we just need to be
// careful about this GC consideration. See WouldGCHelp and
// WasmCodeManager::Commit.
class V8_EXPORT_PRIVATE NativeModule final {
public:
WasmCode* AddCode(const CodeDesc& desc, uint32_t frame_count, uint32_t index,
......@@ -450,7 +445,6 @@ class V8_EXPORT_PRIVATE WasmCodeManager final {
void FreeNativeModule(NativeModule*);
void Free(VirtualMemory* mem);
void AssignRanges(Address start, Address end, NativeModule*);
bool WouldGCHelp() const;
std::map<Address, std::pair<Address, NativeModule*>> lookup_map_;
// Count of NativeModules not yet collected. Helps determine if it's
......
......@@ -43,12 +43,6 @@
'tests/f64': [SKIP],
}], # 'arch == s390 or arch == s390x'
['variant == stress', {
# TODO(v8:7845): Some spec tests currently fail with OOM. Reenable after fixing.
'tests/f32_cmp': [SKIP],
'tests/f64_cmp': [SKIP],
}], # variant == stress
[ALWAYS, {
# TODO(v8:7846): These tests will pass when the wasm-spec-tests repo is updated.
'tests/linking': [SKIP],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment