[wasm] Remove one {NewNativeModule} method

This makes the {code_size_estimate} computation explicit in the caller,
and removes one of the two {NewNativeModule} constructors. It turns out
that the calculation is totally off in the streaming calculation phase,
since no function bodies have been parsed yet. So all
{WasmFunction::code} fields are still empty, and we compute an estimate
that is way too low.
This CL prepares the actual fix for that (by computing a better estimate
at specific call sites).

R=ahaas@chromium.org

Bug: v8:9950
Change-Id: I68a891c97e5f65a9c7e73e21684bdfa7e261e216
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1901273
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64845}
This commit is contained in:
Clemens Backes 2019-11-07 18:16:45 +01:00 committed by Commit Bot
parent 43ad81f36b
commit dde3166beb
11 changed files with 34 additions and 35 deletions

View File

@ -1365,8 +1365,10 @@ std::shared_ptr<NativeModule> CompileToNativeModule(
OwnedVector<uint8_t>::Of(wire_bytes.module_bytes());
// Create a new {NativeModule} first.
size_t code_size_estimate =
wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module.get());
auto native_module = isolate->wasm_engine()->NewNativeModule(
isolate, enabled, std::move(module));
isolate, enabled, std::move(module), code_size_estimate);
native_module->SetWireBytes(std::move(wire_bytes_copy));
CompileNativeModule(isolate, thrower, wasm_module, native_module.get());
@ -1498,8 +1500,10 @@ void AsyncCompileJob::CreateNativeModule(
// breakpoints on a (potentially empty) subset of the instances.
// Create the module object.
size_t code_size_estimate =
wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module.get());
native_module_ = isolate_->wasm_engine()->NewNativeModule(
isolate_, enabled_features_, std::move(module));
isolate_, enabled_features_, std::move(module), code_size_estimate);
native_module_->SetWireBytes({std::move(bytes_copy_), wire_bytes_.length()});
if (stream_) stream_->NotifyNativeModuleCreated(native_module_);

View File

@ -1555,7 +1555,7 @@ size_t WasmCodeManager::EstimateNativeModuleCodeSize(const WasmModule* module) {
}
// static
size_t WasmCodeManager::EstimateNativeModuleNonCodeSize(
size_t WasmCodeManager::EstimateNativeModuleMetaDataSize(
const WasmModule* module) {
size_t wasm_module_estimate = EstimateStoredSize(module);

View File

@ -692,8 +692,11 @@ class V8_EXPORT_PRIVATE WasmCodeManager final {
return total_committed_code_space_.load();
}
// Estimate the needed code space from a completely decoded module.
static size_t EstimateNativeModuleCodeSize(const WasmModule* module);
static size_t EstimateNativeModuleNonCodeSize(const WasmModule* module);
// Estimate the size of meta data needed for the NativeModule, excluding
// generated code. This data still be stored on the C++ heap.
static size_t EstimateNativeModuleMetaDataSize(const WasmModule* module);
private:
friend class WasmCodeAllocator;

View File

@ -673,18 +673,11 @@ void WasmEngine::LogOutstandingCodesForIsolate(Isolate* isolate) {
std::shared_ptr<NativeModule> WasmEngine::NewNativeModule(
Isolate* isolate, const WasmFeatures& enabled,
std::shared_ptr<const WasmModule> module) {
size_t code_size_estimate =
wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module.get());
return NewNativeModule(isolate, enabled, code_size_estimate,
!wasm::NativeModule::kNeedsFarJumpsBetweenCodeSpaces ||
FLAG_wasm_far_jump_table,
std::move(module));
}
std::shared_ptr<NativeModule> WasmEngine::NewNativeModule(
Isolate* isolate, const WasmFeatures& enabled, size_t code_size_estimate,
bool can_request_more, std::shared_ptr<const WasmModule> module) {
std::shared_ptr<const WasmModule> module, size_t code_size_estimate) {
// TODO(clemensb): Remove --wasm-far-jump-table and {can_request_more}.
bool can_request_more =
!wasm::NativeModule::kNeedsFarJumpsBetweenCodeSpaces ||
FLAG_wasm_far_jump_table;
std::shared_ptr<NativeModule> native_module =
code_manager_.NewNativeModule(this, isolate, enabled, code_size_estimate,
can_request_more, std::move(module));

View File

@ -175,16 +175,12 @@ class V8_EXPORT_PRIVATE WasmEngine {
// Create a new NativeModule. The caller is responsible for its
// lifetime. The native module will be given some memory for code,
// which will be page size aligned. The size of the initial memory
// is determined with a heuristic based on the total size of wasm
// code. The native module may later request more memory.
// TODO(titzer): isolate is only required here for CompilationState.
// is determined by {code_size_estimate}. The native module may later request
// more memory.
// TODO(wasm): isolate is only required here for CompilationState.
std::shared_ptr<NativeModule> NewNativeModule(
Isolate* isolate, const WasmFeatures& enabled_features,
std::shared_ptr<const WasmModule> module);
std::shared_ptr<NativeModule> NewNativeModule(
Isolate* isolate, const WasmFeatures& enabled_features,
size_t code_size_estimate, bool can_request_more,
std::shared_ptr<const WasmModule> module);
std::shared_ptr<const WasmModule> module, size_t code_size_estimate);
void FreeNativeModule(NativeModule*);

View File

@ -209,7 +209,7 @@ Handle<WasmModuleObject> WasmModuleObject::New(
// allocating a new {Managed<T>} that the {WasmModuleObject} references.
size_t memory_estimate =
code_size_estimate +
wasm::WasmCodeManager::EstimateNativeModuleNonCodeSize(module);
wasm::WasmCodeManager::EstimateNativeModuleMetaDataSize(module);
Handle<Managed<wasm::NativeModule>> managed_native_module =
Managed<wasm::NativeModule>::FromSharedPtr(isolate, memory_estimate,
std::move(native_module));
@ -2032,7 +2032,7 @@ Handle<AsmWasmData> AsmWasmData::New(
const WasmModule* module = native_module->module();
size_t memory_estimate =
wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module) +
wasm::WasmCodeManager::EstimateNativeModuleNonCodeSize(module);
wasm::WasmCodeManager::EstimateNativeModuleMetaDataSize(module);
Handle<Managed<wasm::NativeModule>> managed_native_module =
Managed<wasm::NativeModule>::FromSharedPtr(isolate, memory_estimate,
std::move(native_module));

View File

@ -614,13 +614,15 @@ MaybeHandle<WasmModuleObject> DeserializeNativeModule(
false, i::wasm::kWasmOrigin, isolate->counters(),
isolate->wasm_engine()->allocator());
if (decode_result.failed()) return {};
CHECK_NOT_NULL(decode_result.value());
WasmModule* module = decode_result.value().get();
std::shared_ptr<WasmModule> module = std::move(decode_result.value());
CHECK_NOT_NULL(module);
Handle<Script> script = CreateWasmScript(
isolate, wire_bytes, module->source_map_url, module->name);
size_t code_size_estimate =
wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module.get());
auto shared_native_module = isolate->wasm_engine()->NewNativeModule(
isolate, enabled_features, std::move(decode_result.value()));
isolate, enabled_features, std::move(module), code_size_estimate);
shared_native_module->SetWireBytes(OwnedVector<uint8_t>::Of(wire_bytes_vec));
Handle<FixedArray> export_wrappers;

View File

@ -126,7 +126,7 @@ std::shared_ptr<wasm::NativeModule> AllocateNativeModule(Isolate* isolate,
// WasmCallDescriptor assumes that code is on the native heap and not
// within a code object.
return isolate->wasm_engine()->NewNativeModule(
isolate, wasm::kAllWasmFeatures, code_size, false, std::move(module));
isolate, wasm::kAllWasmFeatures, std::move(module), code_size);
}
void TestReturnMultipleValues(MachineType type) {

View File

@ -20,10 +20,9 @@ namespace test_wasm_import_wrapper_cache {
std::shared_ptr<NativeModule> NewModule(Isolate* isolate) {
std::shared_ptr<WasmModule> module(new WasmModule);
bool can_request_more = false;
size_t size = 16384;
constexpr size_t kCodeSizeEstimate = 16384;
return isolate->wasm_engine()->NewNativeModule(
isolate, kAllWasmFeatures, size, can_request_more, std::move(module));
isolate, kAllWasmFeatures, std::move(module), kCodeSizeEstimate);
}
TEST(CacheHit) {

View File

@ -318,8 +318,10 @@ Handle<WasmInstanceObject> TestingModuleBuilder::InitInstanceObject() {
isolate_->factory()->NewScript(isolate_->factory()->empty_string());
script->set_type(Script::TYPE_WASM);
size_t code_size_estimate =
wasm::WasmCodeManager::EstimateNativeModuleCodeSize(test_module_.get());
auto native_module = isolate_->wasm_engine()->NewNativeModule(
isolate_, enabled_features_, test_module_);
isolate_, enabled_features_, test_module_, code_size_estimate);
native_module->SetWireBytes(OwnedVector<const uint8_t>());
Handle<WasmModuleObject> module_object =

View File

@ -143,7 +143,7 @@ std::shared_ptr<wasm::NativeModule> AllocateNativeModule(i::Isolate* isolate,
// WasmCallDescriptor assumes that code is on the native heap and not
// within a code object.
return isolate->wasm_engine()->NewNativeModule(
isolate, i::wasm::kAllWasmFeatures, code_size, false, std::move(module));
isolate, i::wasm::kAllWasmFeatures, std::move(module), code_size);
}
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {