Commit e4b585ea authored by Michael Lippautz's avatar Michael Lippautz Committed by V8 LUCI CQ

cppgc: Revert diagnosing CHECKs for Persistent

This CL reverts two diagnosing CLs that introduced same-thread CHECKS,
recovering all introduced performance regressions.

We will try to add less performance-sensitive checks again in a follow
up.

This reverts commit 0c2bbfd5.
This reverts commit 6643c059.

Bug: chromium:1253650, chromium:1243257, chromium:1274201
Change-Id: I96c41c39c4f58b062574fa11c4a2d76ad030bcf7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3315437
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: 's avatarAnton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78245}
parent 9c75acec
...@@ -141,18 +141,18 @@ class V8_EXPORT PersistentRegion final : public PersistentRegionBase { ...@@ -141,18 +141,18 @@ class V8_EXPORT PersistentRegion final : public PersistentRegionBase {
PersistentRegion& operator=(const PersistentRegion&) = delete; PersistentRegion& operator=(const PersistentRegion&) = delete;
V8_INLINE PersistentNode* AllocateNode(void* owner, TraceCallback trace) { V8_INLINE PersistentNode* AllocateNode(void* owner, TraceCallback trace) {
CPPGC_CHECK(IsCreationThread()); CPPGC_DCHECK(IsCreationThread());
return PersistentRegionBase::AllocateNode(owner, trace); return PersistentRegionBase::AllocateNode(owner, trace);
} }
V8_INLINE void FreeNode(PersistentNode* node) { V8_INLINE void FreeNode(PersistentNode* node) {
CPPGC_CHECK(IsCreationThread()); CPPGC_DCHECK(IsCreationThread());
PersistentRegionBase::FreeNode(node); PersistentRegionBase::FreeNode(node);
} }
private:
bool IsCreationThread(); bool IsCreationThread();
private:
int creation_thread_id_; int creation_thread_id_;
}; };
......
...@@ -189,15 +189,6 @@ class BasicPersistent final : public PersistentBase, ...@@ -189,15 +189,6 @@ class BasicPersistent final : public PersistentBase,
// heterogeneous assignments between different Member and Persistent handles // heterogeneous assignments between different Member and Persistent handles
// based on their actual types. // based on their actual types.
V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const { V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const {
// TODO(chromium:1253650): Temporary CHECK to diagnose issues.
if (IsValid()) {
CPPGC_CHECK(
WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
CPPGC_CHECK(GetNode() != nullptr);
} else {
CPPGC_CHECK(GetNode() == nullptr);
}
// The const_cast below removes the constness from PersistentBase storage. // The const_cast below removes the constness from PersistentBase storage.
// The following static_cast re-adds any constness if specified through the // The following static_cast re-adds any constness if specified through the
// user-visible template parameter T. // user-visible template parameter T.
...@@ -205,14 +196,6 @@ class BasicPersistent final : public PersistentBase, ...@@ -205,14 +196,6 @@ class BasicPersistent final : public PersistentBase,
} }
void Clear() { void Clear() {
// TODO(chromium:1253650): Temporary CHECK to diagnose issues.
if (IsValid()) {
CPPGC_CHECK(
WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
CPPGC_CHECK(GetNode() != nullptr);
} else {
CPPGC_CHECK(GetNode() == nullptr);
}
// Simplified version of `Assign()` to allow calling without a complete type // Simplified version of `Assign()` to allow calling without a complete type
// `T`. // `T`.
if (IsValid()) { if (IsValid()) {
......
...@@ -103,19 +103,6 @@ void PersistentRegionBase::Trace(Visitor* visitor) { ...@@ -103,19 +103,6 @@ void PersistentRegionBase::Trace(Visitor* visitor) {
nodes_.end()); nodes_.end());
} }
namespace {
thread_local int thread_id = 0;
int GetCurrentThreadId() {
if (thread_id == 0) {
thread_id = v8::base::OS::GetCurrentThreadId();
}
return thread_id;
}
} // namespace
PersistentRegion::PersistentRegion(const FatalOutOfMemoryHandler& oom_handler) PersistentRegion::PersistentRegion(const FatalOutOfMemoryHandler& oom_handler)
: PersistentRegionBase(oom_handler), : PersistentRegionBase(oom_handler),
creation_thread_id_(v8::base::OS::GetCurrentThreadId()) { creation_thread_id_(v8::base::OS::GetCurrentThreadId()) {
...@@ -123,10 +110,7 @@ PersistentRegion::PersistentRegion(const FatalOutOfMemoryHandler& oom_handler) ...@@ -123,10 +110,7 @@ PersistentRegion::PersistentRegion(const FatalOutOfMemoryHandler& oom_handler)
} }
bool PersistentRegion::IsCreationThread() { bool PersistentRegion::IsCreationThread() {
// Short circuit using TLS cache. If that doesn't work (e.g. in TLS teardown) return creation_thread_id_ == v8::base::OS::GetCurrentThreadId();
// fall back to the more expensive call in base.
return creation_thread_id_ == GetCurrentThreadId() ||
creation_thread_id_ == v8::base::OS::GetCurrentThreadId();
} }
PersistentRegionLock::PersistentRegionLock() { PersistentRegionLock::PersistentRegionLock() {
......
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