From 24ca73004e7587cfe3cf2c0116a363aab8d3000d Mon Sep 17 00:00:00 2001 From: Michael Lippautz Date: Wed, 11 Jan 2023 14:12:51 +0100 Subject: [PATCH] cppgc: Implement slim write barrier Introduces a slim write barrier for Oilpan behind `cppgc_enable_slim_write_barrier` that is enabled by default. The slim write barrier only performs a single approximate global check for whether the write barrier is needed and delegates all other checks to a slow path call. This is beneficial in configurations that do not need many checks for the barrier overall, i.e., configurations without young generation. Young generation is off by default which is why this approach is beneficial. On Speedometer the write barrier is hit 75M times with a fast bailout of 99.3%. Progression on Speedometer2 is somewhere around 0.2-0.5%. The resulting code embedded in another function is only 34 bytes compared to 128 bytes before. See attached bug for detailed assembly snippet. Change-Id: I6869513186e7a26104c46f1f2ac2cfa855689f64 Bug: chromium:1406464 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4152488 Reviewed-by: Anton Bikineev Commit-Queue: Michael Lippautz Cr-Commit-Position: refs/heads/main@{#85232} --- BUILD.bazel | 1 + BUILD.gn | 4 +++ gni/v8.gni | 6 +++++ include/cppgc/internal/pointer-policies.h | 10 +++++++ include/cppgc/internal/write-barrier.h | 7 +++++ src/heap/cppgc/write-barrier.cc | 33 +++++++++++++++++++++++ 6 files changed, 61 insertions(+) diff --git a/BUILD.bazel b/BUILD.bazel index 8efca775f3..81ee95c4d8 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -68,6 +68,7 @@ load(":bazel/v8-non-pointer-compression.bzl", "v8_binary_non_pointer_compression # v8_enable_sandbox # cppgc_enable_caged_heap # cppgc_enable_check_assignments_in_prefinalizers +# cppgc_enable_slim_write_barrier # cppgc_enable_object_names # cppgc_enable_pointer_compression # cppgc_enable_verify_heap diff --git a/BUILD.gn b/BUILD.gn index f121b98733..1a5487287f 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -840,6 +840,7 @@ config("v8_header_features") { external_cppgc_defines = [ "CPPGC_SUPPORTS_OBJECT_NAMES", "CPPGC_CAGED_HEAP", + "CPPGC_SLIM_WRITE_BARRIER", "CPPGC_YOUNG_GENERATION", "CPPGC_POINTER_COMPRESSION", ] @@ -869,6 +870,9 @@ if (cppgc_enable_pointer_compression) { if (cppgc_enable_2gb_cage) { enabled_external_cppgc_defines += [ "CPPGC_2GB_CAGE" ] } +if (cppgc_enable_slim_write_barrier) { + enabled_external_cppgc_defines += [ "CPPGC_SLIM_WRITE_BARRIER" ] +} disabled_external_cppgc_defines = external_cppgc_defines - enabled_external_cppgc_defines diff --git a/gni/v8.gni b/gni/v8.gni index accae349a9..5215b81416 100644 --- a/gni/v8.gni +++ b/gni/v8.gni @@ -106,6 +106,12 @@ declare_args() { # Enable young generation in cppgc. cppgc_enable_young_generation = false + # Enables a slim write barrier that only performs a single check in the fast + # path and delegates all further checks to a slow path call. This is fast + # in a setting with few slow-path checks, i.e., with disabled young generation + # GC. + cppgc_enable_slim_write_barrier = true + # Enable pointer compression in cppgc. cppgc_enable_pointer_compression = false diff --git a/include/cppgc/internal/pointer-policies.h b/include/cppgc/internal/pointer-policies.h index 8455b3df81..e67da040db 100644 --- a/include/cppgc/internal/pointer-policies.h +++ b/include/cppgc/internal/pointer-policies.h @@ -34,18 +34,28 @@ struct DijkstraWriteBarrierPolicy { } V8_INLINE static void AssigningBarrier(const void* slot, const void* value) { +#ifdef CPPGC_SLIM_WRITE_BARRIER + if (V8_UNLIKELY(WriteBarrier::IsEnabled())) + WriteBarrier::CombinedWriteBarrierSlow(slot); +#else // !CPPGC_SLIM_WRITE_BARRIER WriteBarrier::Params params; const WriteBarrier::Type type = WriteBarrier::GetWriteBarrierType(slot, value, params); WriteBarrier(type, params, slot, value); +#endif // !CPPGC_SLIM_WRITE_BARRIER } V8_INLINE static void AssigningBarrier(const void* slot, MemberStorage storage) { +#ifdef CPPGC_SLIM_WRITE_BARRIER + if (V8_UNLIKELY(WriteBarrier::IsEnabled())) + WriteBarrier::CombinedWriteBarrierSlow(slot); +#else // !CPPGC_SLIM_WRITE_BARRIER WriteBarrier::Params params; const WriteBarrier::Type type = WriteBarrier::GetWriteBarrierType(slot, storage, params); WriteBarrier(type, params, slot, storage.Load()); +#endif // !CPPGC_SLIM_WRITE_BARRIER } private: diff --git a/include/cppgc/internal/write-barrier.h b/include/cppgc/internal/write-barrier.h index 37bc5c973e..3e4440df2a 100644 --- a/include/cppgc/internal/write-barrier.h +++ b/include/cppgc/internal/write-barrier.h @@ -79,6 +79,13 @@ class V8_EXPORT WriteBarrier final { // Returns the required write barrier for a given `value`. static V8_INLINE Type GetWriteBarrierType(const void* value, Params& params); +#ifdef CPPGC_SLIM_WRITE_BARRIER + // A write barrier that combines `GenerationalBarrier()` and + // `DijkstraMarkingBarrier()`. We only pass a single parameter here to clobber + // as few registers as possible. + static V8_NOINLINE void CombinedWriteBarrierSlow(const void* slot); +#endif // CPPGC_SLIM_WRITE_BARRIER + static V8_INLINE void DijkstraMarkingBarrier(const Params& params, const void* object); static V8_INLINE void DijkstraMarkingBarrierRange( diff --git a/src/heap/cppgc/write-barrier.cc b/src/heap/cppgc/write-barrier.cc index 5cbec656a9..d7aa1284eb 100644 --- a/src/heap/cppgc/write-barrier.cc +++ b/src/heap/cppgc/write-barrier.cc @@ -5,6 +5,7 @@ #include "src/heap/cppgc/write-barrier.h" #include "include/cppgc/heap-consistency.h" +#include "include/cppgc/internal/member-storage.h" #include "include/cppgc/internal/pointer-policies.h" #include "src/heap/cppgc/globals.h" #include "src/heap/cppgc/heap-object-header.h" @@ -222,5 +223,37 @@ bool YoungGenerationEnabler::IsEnabled() { #endif // defined(CPPGC_YOUNG_GENERATION) +#ifdef CPPGC_SLIM_WRITE_BARRIER + +// static +void WriteBarrier::CombinedWriteBarrierSlow(const void* slot) { +#if defined(CPPGC_POINTER_COMPRESSION) + using MemberStorage = CompressedPointer; +#else // !defined(CPPGC_POINTER_COMPRESSION) + using MemberStorage = RawPointer; +#endif // !defined(CPPGC_POINTER_COMPRESSION) + const auto* storage = reinterpret_cast(slot); + const void* value = storage->Load(); + WriteBarrier::Params params; + const WriteBarrier::Type type = + WriteBarrier::GetWriteBarrierType(slot, value, params); + switch (type) { + case WriteBarrier::Type::kGenerational: + WriteBarrier::GenerationalBarrier< + WriteBarrier::GenerationalBarrierType::kPreciseSlot>(params, slot); + break; + case WriteBarrier::Type::kMarking: + WriteBarrier::DijkstraMarkingBarrier(params, value); + break; + case WriteBarrier::Type::kNone: + // The fast checks are approximate and may trigger spuriously if any heap + // has marking in progress. `GetWriteBarrierType()` above is exact which + // is the reason we could still observe a bailout here. + break; + } +} + +#endif // CPPGC_SLIM_WRITE_BARRIER + } // namespace internal } // namespace cppgc