Commit 0e6eb3e2 authored by Andreas Haas's avatar Andreas Haas Committed by V8 LUCI CQ

[wasm] Allow dynamic tiering to release CompilationEventCallbacks

With eager compilation, CompilationEventCallbacks get released when all
compilation units in the compilation state are finished. This is
possible because no future compilation event could get triggered after
that. With dynamic tiering, though, the {FinishedCompilationChunk} event
can trigger repeatedly, even after all compilation units finish at some
point in time, as dynamic tiering can create new CompilationUnits. As
a temporary fix, CompilationEventCallbacks don't get released when
dynamic tiering is enabled.

This CL fixes this issue by turning the callback from an std::function
into a class, and adding a second function to the class which indicates
whether the callback can be released when all compilation units in the
compilation state are finished. Thereby all callbacks can be deallocated
except the ones like the code caching callback which waits for the
{FinishedCompilationChunk} events.

R=jkummerow@chromium.org

Bug: v8:12289
Change-Id: I0f73f4bd2dffe644c9a26c274cb52ac6fa49ab67
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3264288
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77890}
parent b927dc15
......@@ -117,6 +117,23 @@ enum class CompilationEvent : uint8_t {
kFinishedRecompilation
};
class V8_EXPORT_PRIVATE CompilationEventCallback {
public:
virtual ~CompilationEventCallback() = default;
virtual void call(CompilationEvent event) = 0;
enum class ReleaseAfterFinalEvent { kRelease, kKeep };
// Tells the module compiler whether to keep or to release a callback when the
// compilation state finishes all compilation units. Most callbacks should be
// released, that's why there is a default implementation, but the callback
// for code caching with dynamic tiering has to stay alive.
virtual ReleaseAfterFinalEvent release_after_final_event() {
return ReleaseAfterFinalEvent::kRelease;
}
};
// The implementation of {CompilationState} lives in module-compiler.cc.
// This is the PIMPL interface to that private class.
class V8_EXPORT_PRIVATE CompilationState {
......@@ -137,7 +154,7 @@ class V8_EXPORT_PRIVATE CompilationState {
std::shared_ptr<WireBytesStorage> GetWireBytesStorage() const;
void AddCallback(callback_t);
void AddCallback(std::unique_ptr<CompilationEventCallback> callback);
void InitializeAfterDeserialization(
base::Vector<const int> missing_functions);
......
This diff is collapsed.
......@@ -312,7 +312,7 @@ void AsyncStreamingDecoder::Abort() {
namespace {
class CompilationChunkFinishedCallback {
class CompilationChunkFinishedCallback : public CompilationEventCallback {
public:
CompilationChunkFinishedCallback(
std::weak_ptr<NativeModule> native_module,
......@@ -320,7 +320,7 @@ class CompilationChunkFinishedCallback {
: native_module_(std::move(native_module)),
callback_(std::move(callback)) {}
void operator()(CompilationEvent event) const {
void call(CompilationEvent event) override {
if (event != CompilationEvent::kFinishedCompilationChunk &&
event != CompilationEvent::kFinishedTopTierCompilation) {
return;
......@@ -332,6 +332,10 @@ class CompilationChunkFinishedCallback {
}
}
ReleaseAfterFinalEvent release_after_final_event() override {
return CompilationEventCallback::ReleaseAfterFinalEvent::kKeep;
}
private:
const std::weak_ptr<NativeModule> native_module_;
const AsyncStreamingDecoder::ModuleCompiledCallback callback_;
......@@ -343,8 +347,9 @@ void AsyncStreamingDecoder::NotifyNativeModuleCreated(
const std::shared_ptr<NativeModule>& native_module) {
if (!module_compiled_callback_) return;
auto* comp_state = native_module->compilation_state();
comp_state->AddCallback(CompilationChunkFinishedCallback{
std::move(native_module), std::move(module_compiled_callback_)});
comp_state->AddCallback(std::make_unique<CompilationChunkFinishedCallback>(
std::move(native_module), std::move(module_compiled_callback_)));
module_compiled_callback_ = {};
}
......
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