[wasm][lazy] Avoid validation in case of a prefix_cache_hit

If the same WebAssembly module gets compiled multiple times, the
compilation result of the first compilation gets reused for later
compilations. With streaming compilation functions get compiled before
the whole module got downloaded, so it cannot be determined if the
currently compiled module has already been compiled or not. Therefore,
to check if the WebAssembly module has already been compiled, we compare
if the hash of the header section matches the hash of any of the already
compiled modules. If so, no function gets compiled until all bytes were
received. Then a full module check can be done, and either an existing
module can be reused, or the whole module gets compiled.

While compilation is avoided after a prefix_cache_hit, decoding still has
to happen. In the existing implementation, validation for lazy
compilation also happened in addition to decoding. This lead to the
problem that validation of lazy compilation could post a foreground task
when an error was detected, and later another foreground task got posted
when all bytes were received to do the full module check. Having two
foreground tasks at the same time violates an invariant in the
AsyncCompileJob.

With this CL we avoid the initial function validation after a
prefix_cache_hit to avoid the task for the error handling. Validation
will anyways happen again if the full module check fails later, or
validation is unnecessary if the full module check succeeds, as the
module has already been validated before.

R=clemensb@chromium.org

Bug: v8:13147, v8:12852
Change-Id: Iae24c056057f3a5dfd2f61accd1f9f0d35412996
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3812038
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82218}

src/wasm/module-compiler.cc

@@ -2877,18 +2877,25 @@ bool AsyncStreamingProcessor::ProcessCodeSectionHeader(
 void AsyncStreamingProcessor::ProcessFunctionBody(
     base::Vector<const uint8_t> bytes, uint32_t offset) {
   TRACE_STREAMING("Process function body %d ...\n", num_functions_);
-  // We first have to check that the compilation state exists, because with a
-  // prefix_cache_hit_ it would not exist.
-  if (job_->native_module_ &&
-      job_->native_module_->compilation_state()->failed()) {
+  // In case of {prefix_cache_hit} we still need the function body to be
+  // decoded. Otherwise a later cache miss cannot be handled.
+  decoder_.DecodeFunctionBody(
+      num_functions_, static_cast<uint32_t>(bytes.length()), offset, false);
+
+  if (prefix_cache_hit_) {
+    // Don't compile yet if we might have a cache hit.
+    ++num_functions_;
+    return;
+  }
+
+  // Bail out after the {prefix_cache_hit_}, because if {prefix_cache_hit_} is
+  // true, the native module does not exist.
+  if (job_->native_module_->compilation_state()->failed()) {
     // There has already been an error, there is no need to do any more
     // validation or compiling.
     return;
   }
 
-  decoder_.DecodeFunctionBody(
-      num_functions_, static_cast<uint32_t>(bytes.length()), offset, false);
-
   const WasmModule* module = decoder_.module();
   auto enabled_features = job_->enabled_features_;
   uint32_t func_index =
@@ -2913,12 +2920,6 @@ void AsyncStreamingProcessor::ProcessFunctionBody(
     }
   }
 
-  // Don't compile yet if we might have a cache hit.
-  if (prefix_cache_hit_) {
-    ++num_functions_;
-    return;
-  }
-
   auto* compilation_state = Impl(job_->native_module_->compilation_state());
   compilation_state->AddCompilationUnit(compilation_unit_builder_.get(),
                                         func_index);