Skip to content

Commit f942a49

Browse files
nickieV8 LUCI CQ
nickie
authored and
V8 LUCI CQ
committed
[handles] Clean up direct handle configurations
This CL merges the direct handle and direct local configurations, that were until now behind two different compile-time flags and two different macros. V8_ENABLE_DIRECT_HANDLE is now used to control both. It is set with v8_enable_direct_handle=true and (unless overriden) it is also implied by v8_enable_conservative_stack_scanning=true. The compile-time flag and macro for direct locals are removed. Bug: 41480448 Bug: 42203211 Change-Id: I358495659fa7d2c6dfbd8c36e016ad75d3b3fcaa Cq-Include-Trybots: luci.v8.try:v8_linux64_css_dbg Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5736205 Reviewed-by: Omer Katz <[email protected]> Reviewed-by: Michael Lippautz <[email protected]> Commit-Queue: Nikolaos Papaspyrou <[email protected]> Cr-Commit-Position: refs/heads/main@{#95297}
1 parent d57d081 commit f942a49

36 files changed

Lines changed: 68 additions & 91 deletions

‎BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ load(":bazel/v8-non-pointer-compression.bzl", "v8_binary_non_pointer_compression
4343
# v8_enable_concurrent_marking
4444
# v8_enable_conservative_stack_scanning
4545
# v8_enable_direct_handle
46-
# v8_enable_direct_local
4746
# v8_enable_local_off_stack_check
4847
# v8_enable_ignition_dispatch_counting
4948
# v8_enable_builtins_optimization

‎BUILD.gn‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,7 @@ external_v8_defines = [
896896
"V8_MAP_PACKING",
897897
"V8_IS_TSAN",
898898
"V8_ENABLE_CONSERVATIVE_STACK_SCANNING",
899-
"V8_ENABLE_DIRECT_LOCAL",
899+
"V8_ENABLE_DIRECT_HANDLE",
900900
"V8_MINORMS_STRING_SHORTCUTTING",
901901
"V8_HAVE_TARGET_OS",
902902
"V8_TARGET_OS_ANDROID",
@@ -950,8 +950,8 @@ if (is_tsan) {
950950
if (v8_enable_conservative_stack_scanning) {
951951
enabled_external_v8_defines += [ "V8_ENABLE_CONSERVATIVE_STACK_SCANNING" ]
952952
}
953-
if (v8_enable_direct_local) {
954-
enabled_external_v8_defines += [ "V8_ENABLE_DIRECT_LOCAL" ]
953+
if (v8_enable_direct_handle) {
954+
enabled_external_v8_defines += [ "V8_ENABLE_DIRECT_HANDLE" ]
955955
}
956956
if (v8_shortcut_strings_in_minor_ms) {
957957
enabled_external_v8_defines += [ "V8_MINORMS_STRING_SHORTCUTTING" ]
@@ -1310,9 +1310,6 @@ config("features") {
13101310
if (v8_enable_builtin_jump_table_switch) {
13111311
defines += [ "V8_ENABLE_BUILTIN_JUMP_TABLE_SWITCH" ]
13121312
}
1313-
if (v8_enable_direct_handle) {
1314-
defines += [ "V8_ENABLE_DIRECT_HANDLE" ]
1315-
}
13161313
if (v8_enable_extensible_ro_snapshot) {
13171314
defines += [ "V8_ENABLE_EXTENSIBLE_RO_SNAPSHOT" ]
13181315
}
@@ -2742,7 +2739,6 @@ action("v8_dump_build_config") {
27422739
"debugging_features=$v8_enable_debugging_features",
27432740
"dict_property_const_tracking=$v8_dict_property_const_tracking",
27442741
"direct_handle=$v8_enable_direct_handle",
2745-
"direct_local=$v8_enable_direct_local",
27462742
"disassembler=$v8_enable_disassembler",
27472743
"full_debug=$is_full_debug",
27482744
"gdbjit=$v8_enable_gdbjit",
@@ -2800,7 +2796,6 @@ generated_file("v8_generate_features_json") {
28002796
v8_enable_conservative_stack_scanning =
28012797
v8_enable_conservative_stack_scanning
28022798
v8_enable_direct_handle = v8_enable_direct_handle
2803-
v8_enable_direct_local = v8_enable_direct_local
28042799
v8_enable_extensible_ro_snapshot = v8_enable_extensible_ro_snapshot
28052800
v8_enable_gdbjit = v8_enable_gdbjit
28062801
v8_enable_hugepage = v8_enable_hugepage

‎bazel/defs.bzl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,6 @@ def build_config_content(cpu, icu):
549549
("debugging_features", "false"),
550550
("dict_property_const_tracking", "false"),
551551
("direct_handle", "false"),
552-
("direct_local", "false"),
553552
("disassembler", "false"),
554553
("full_debug", "false"),
555554
("gdbjit", "false"),

‎gni/v8.gni‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,9 @@ declare_args() {
118118
# Scan the call stack conservatively during garbage collection.
119119
v8_enable_conservative_stack_scanning = false
120120

121-
# Use direct pointers in internal (direct) handles.
121+
# Use direct pointers in handles (v8::internal::Handle and v8::Local).
122122
v8_enable_direct_handle = ""
123123

124-
# Use direct pointers in local handles.
125-
v8_enable_direct_local = ""
126-
127124
# Check for off-stack allocated local handles.
128125
v8_enable_local_off_stack_check = false
129126

@@ -238,9 +235,6 @@ assert(v8_enable_turbofan || !v8_enable_webassembly,
238235
if (v8_enable_direct_handle == "") {
239236
v8_enable_direct_handle = v8_enable_conservative_stack_scanning
240237
}
241-
if (v8_enable_direct_local == "") {
242-
v8_enable_direct_local = v8_enable_conservative_stack_scanning
243-
}
244238

245239
# Points to // in v8 stand-alone or to //v8/ in chromium. We need absolute
246240
# paths for all configs in templates as they are shared in different

‎include/v8-handle-base.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class IndirectHandleBase {
9090
internal::Address* location_ = nullptr;
9191
};
9292

93-
#ifdef V8_ENABLE_DIRECT_LOCAL
93+
#ifdef V8_ENABLE_DIRECT_HANDLE
9494

9595
/**
9696
* A base class for abstract handles containing direct pointers.
@@ -130,7 +130,7 @@ class DirectHandleBase {
130130
internal::Address ptr_ = internal::ValueHelper::kEmpty;
131131
};
132132

133-
#endif // V8_ENABLE_DIRECT_LOCAL
133+
#endif // V8_ENABLE_DIRECT_HANDLE
134134

135135
} // namespace v8::api_internal
136136

‎include/v8-internal.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,12 +1557,12 @@ constexpr WrappedIterator<Iterator> operator+(
15571557
// whether direct local support is enabled.
15581558
class ValueHelper final {
15591559
public:
1560-
#ifdef V8_ENABLE_DIRECT_LOCAL
1560+
#ifdef V8_ENABLE_DIRECT_HANDLE
15611561
static constexpr Address kTaggedNullAddress = 1;
15621562
static constexpr Address kEmpty = kTaggedNullAddress;
15631563
#else
15641564
static constexpr Address kEmpty = kNullAddress;
1565-
#endif // V8_ENABLE_DIRECT_LOCAL
1565+
#endif // V8_ENABLE_DIRECT_HANDLE
15661566

15671567
template <typename T>
15681568
V8_INLINE static bool IsEmpty(T* value) {
@@ -1578,7 +1578,7 @@ class ValueHelper final {
15781578
return handle.template value<T>();
15791579
}
15801580

1581-
#ifdef V8_ENABLE_DIRECT_LOCAL
1581+
#ifdef V8_ENABLE_DIRECT_HANDLE
15821582

15831583
template <typename T>
15841584
V8_INLINE static Address ValueAsAddress(const T* value) {
@@ -1593,7 +1593,7 @@ class ValueHelper final {
15931593
return *reinterpret_cast<T**>(slot);
15941594
}
15951595

1596-
#else // !V8_ENABLE_DIRECT_LOCAL
1596+
#else // !V8_ENABLE_DIRECT_HANDLE
15971597

15981598
template <typename T>
15991599
V8_INLINE static Address ValueAsAddress(const T* value) {
@@ -1605,7 +1605,7 @@ class ValueHelper final {
16051605
return reinterpret_cast<T*>(slot);
16061606
}
16071607

1608-
#endif // V8_ENABLE_DIRECT_LOCAL
1608+
#endif // V8_ENABLE_DIRECT_HANDLE
16091609
};
16101610

16111611
/**

‎include/v8-local-handle.h‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,11 @@ class V8_EXPORT V8_NODISCARD HandleScope {
150150

151151
/**
152152
* A base class for local handles.
153-
* Its implementation depends on whether direct local support is enabled.
153+
* Its implementation depends on whether direct handle support is enabled.
154154
* When it is, a local handle contains a direct pointer to the referenced
155155
* object, otherwise it contains an indirect pointer.
156156
*/
157-
#ifdef V8_ENABLE_DIRECT_LOCAL
157+
#ifdef V8_ENABLE_DIRECT_HANDLE
158158

159159
template <typename T>
160160
class LocalBase : public api_internal::DirectHandleBase {
@@ -183,7 +183,7 @@ class LocalBase : public api_internal::DirectHandleBase {
183183
}
184184
};
185185

186-
#else // !V8_ENABLE_DIRECT_LOCAL
186+
#else // !V8_ENABLE_DIRECT_HANDLE
187187

188188
template <typename T>
189189
class LocalBase : public api_internal::IndirectHandleBase {
@@ -215,7 +215,7 @@ class LocalBase : public api_internal::IndirectHandleBase {
215215
}
216216
};
217217

218-
#endif // V8_ENABLE_DIRECT_LOCAL
218+
#endif // V8_ENABLE_DIRECT_HANDLE
219219

220220
/**
221221
* An object reference managed by the v8 garbage collector.
@@ -400,13 +400,13 @@ class V8_TRIVIAL_ABI Local : public LocalBase<T>,
400400
return Local<T>(LocalBase<T>::FromSlot(slot));
401401
}
402402

403-
#ifdef V8_ENABLE_DIRECT_LOCAL
403+
#ifdef V8_ENABLE_DIRECT_HANDLE
404404
friend class TypecheckWitness;
405405

406406
V8_INLINE static Local<T> FromAddress(internal::Address ptr) {
407407
return Local<T>(LocalBase<T>(ptr));
408408
}
409-
#endif // V8_ENABLE_DIRECT_LOCAL
409+
#endif // V8_ENABLE_DIRECT_HANDLE
410410

411411
V8_INLINE static Local<T> New(Isolate* isolate, internal::Address value) {
412412
return Local<T>(LocalBase<T>::New(isolate, value));
@@ -444,7 +444,7 @@ class V8_TRIVIAL_ABI LocalUnchecked : public Local<T> {
444444
: Local<T>(other, Local<T>::do_not_check) {}
445445
};
446446

447-
#ifdef V8_ENABLE_DIRECT_LOCAL
447+
#ifdef V8_ENABLE_DIRECT_HANDLE
448448
// Off-stack allocated direct locals must be registered as strong roots.
449449
// For off-stack indirect locals, this is not necessary.
450450

@@ -471,15 +471,15 @@ class StrongRootAllocator<LocalUnchecked<T>> : public StrongRootAllocatorBase {
471471
return deallocate_impl(reinterpret_cast<Address*>(p), n);
472472
}
473473
};
474-
#endif // V8_ENABLE_DIRECT_LOCAL
474+
#endif // V8_ENABLE_DIRECT_HANDLE
475475
} // namespace internal
476476

477477
template <typename T>
478478
class LocalVector {
479479
private:
480480
using element_type = internal::LocalUnchecked<T>;
481481

482-
#ifdef V8_ENABLE_DIRECT_LOCAL
482+
#ifdef V8_ENABLE_DIRECT_HANDLE
483483
using allocator_type = internal::StrongRootAllocator<element_type>;
484484

485485
static allocator_type make_allocator(Isolate* isolate) noexcept {
@@ -491,7 +491,7 @@ class LocalVector {
491491
static allocator_type make_allocator(Isolate* isolate) noexcept {
492492
return allocator_type();
493493
}
494-
#endif // V8_ENABLE_DIRECT_LOCAL
494+
#endif // V8_ENABLE_DIRECT_HANDLE
495495

496496
using vector_type = std::vector<element_type, allocator_type>;
497497

@@ -715,7 +715,7 @@ class V8_EXPORT V8_NODISCARD EscapableHandleScope
715715
V8_INLINE ~EscapableHandleScope() = default;
716716
template <class T>
717717
V8_INLINE Local<T> Escape(Local<T> value) {
718-
#ifdef V8_ENABLE_DIRECT_LOCAL
718+
#ifdef V8_ENABLE_DIRECT_HANDLE
719719
return value;
720720
#else
721721
if (value.IsEmpty()) return value;

‎include/v8-util.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ class PersistentValueMapBase {
182182
*/
183183
Local<V> Get(const K& key) {
184184
V* p = FromVal(Traits::Get(&impl_, key));
185-
#ifdef V8_ENABLE_DIRECT_LOCAL
185+
#ifdef V8_ENABLE_DIRECT_HANDLE
186186
if (p == nullptr) return Local<V>();
187187
#endif
188188
return Local<V>::New(isolate_, p);

‎src/api/api-inl.h‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,21 @@ inline v8::internal::Handle<i::UnionOf<i::Smi, i::Foreign>> FromCData(
5555

5656
template <class From, class To>
5757
inline Local<To> Utils::Convert(v8::internal::Handle<From> obj) {
58-
DCHECK(obj.is_null() || (IsSmi(*obj) || !IsTheHole(*obj)));
59-
#ifdef V8_ENABLE_DIRECT_LOCAL
58+
DCHECK(obj.is_null() || IsSmi(*obj) || !IsTheHole(*obj));
59+
#ifdef V8_ENABLE_DIRECT_HANDLE
6060
if (obj.is_null()) return Local<To>();
6161
#endif
6262
return Local<To>::FromSlot(obj.location());
6363
}
6464

65+
// TODO(42203211): The second parameter (isolate) is not necessary anymore. It
66+
// will be removed in a subsequent CL.
6567
template <class From, class To>
6668
inline Local<To> Utils::Convert(v8::internal::DirectHandle<From> obj,
6769
v8::internal::Isolate* isolate) {
68-
#if defined(V8_ENABLE_DIRECT_LOCAL)
69-
DCHECK(obj.is_null() || (IsSmi(*obj) || !IsTheHole(*obj)));
70+
#if defined(V8_ENABLE_DIRECT_HANDLE)
71+
DCHECK(obj.is_null() || IsSmi(*obj) || !IsTheHole(*obj));
7072
return Local<To>::FromAddress(obj.address());
71-
#elif defined(V8_ENABLE_DIRECT_HANDLE)
72-
if (obj.is_null()) return Local<To>();
73-
return Convert<From, To>(v8::internal::Handle<From>(*obj, isolate));
7473
#else
7574
return Convert<From, To>(obj);
7675
#endif
@@ -113,7 +112,7 @@ TYPED_ARRAYS(MAKE_TO_LOCAL_TYPED_ARRAY)
113112

114113
// Implementations of OpenHandle
115114

116-
#ifdef V8_ENABLE_DIRECT_LOCAL
115+
#ifdef V8_ENABLE_DIRECT_HANDLE
117116

118117
#define MAKE_OPEN_HANDLE(From, To) \
119118
v8::internal::Handle<v8::internal::To> Utils::OpenHandle( \
@@ -145,7 +144,7 @@ TYPED_ARRAYS(MAKE_TO_LOCAL_TYPED_ARRAY)
145144
return Utils::OpenHandle(that, allow_empty_handle); \
146145
}
147146

148-
#else // !V8_ENABLE_DIRECT_LOCAL
147+
#else // !V8_ENABLE_DIRECT_HANDLE
149148

150149
#define MAKE_OPEN_HANDLE(From, To) \
151150
v8::internal::Handle<v8::internal::To> Utils::OpenHandle( \
@@ -169,7 +168,7 @@ TYPED_ARRAYS(MAKE_TO_LOCAL_TYPED_ARRAY)
169168
return Utils::OpenHandle(that, allow_empty_handle); \
170169
}
171170

172-
#endif // V8_ENABLE_DIRECT_LOCAL
171+
#endif // V8_ENABLE_DIRECT_HANDLE
173172

174173
OPEN_HANDLE_LIST(MAKE_OPEN_HANDLE)
175174

@@ -254,7 +253,7 @@ class V8_NODISCARD InternalEscapableScope : public EscapableHandleScopeBase {
254253
*/
255254
template <class T>
256255
V8_INLINE Local<T> Escape(Local<T> value) {
257-
#ifdef V8_ENABLE_DIRECT_LOCAL
256+
#ifdef V8_ENABLE_DIRECT_HANDLE
258257
return value;
259258
#else
260259
DCHECK(!value.IsEmpty());

‎src/api/api.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -923,14 +923,14 @@ i::Address* HandleScope::CreateHandle(i::Isolate* i_isolate, i::Address value) {
923923
return i::HandleScope::CreateHandle(i_isolate, value);
924924
}
925925

926-
#ifdef V8_ENABLE_DIRECT_LOCAL
926+
#ifdef V8_ENABLE_DIRECT_HANDLE
927927

928928
i::Address* HandleScope::CreateHandleForCurrentIsolate(i::Address value) {
929929
i::Isolate* i_isolate = i::Isolate::Current();
930930
return i::HandleScope::CreateHandle(i_isolate, value);
931931
}
932932

933-
#endif // V8_ENABLE_DIRECT_LOCAL
933+
#endif // V8_ENABLE_DIRECT_HANDLE
934934

935935
EscapableHandleScopeBase::EscapableHandleScopeBase(Isolate* v8_isolate) {
936936
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
@@ -5441,7 +5441,7 @@ bool v8::Object::IsUndetectable() const {
54415441
}
54425442

54435443
namespace {
5444-
#ifdef V8_ENABLE_DIRECT_LOCAL
5444+
#ifdef V8_ENABLE_DIRECT_HANDLE
54455445
// A newly allocated vector is required to convert from an array of direct
54465446
// locals to an array of indirect handles.
54475447
std::vector<i::Handle<i::Object>> PrepareArguments(int argc,
@@ -5452,7 +5452,7 @@ std::vector<i::Handle<i::Object>> PrepareArguments(int argc,
54525452
}
54535453
return args;
54545454
}
5455-
#else // !V8_ENABLE_DIRECT_LOCAL
5455+
#else // !V8_ENABLE_DIRECT_HANDLE
54565456
// A simple cast is used to convert from an array of indirect locals to an
54575457
// array of indirect handles. A MemorySpan object is returned, as no
54585458
// deallocation is necessary.
@@ -5461,7 +5461,7 @@ v8::MemorySpan<i::Handle<i::Object>> PrepareArguments(int argc,
54615461
return {reinterpret_cast<i::Handle<i::Object>*>(argv),
54625462
static_cast<size_t>(argc)};
54635463
}
5464-
#endif // V8_ENABLE_DIRECT_LOCAL
5464+
#endif // V8_ENABLE_DIRECT_HANDLE
54655465
} // namespace
54665466

54675467
MaybeLocal<Value> Object::CallAsFunction(Local<Context> context,
@@ -8412,7 +8412,7 @@ Maybe<void> v8::Array::Iterate(Local<Context> context,
84128412
}
84138413

84148414
v8::TypecheckWitness::TypecheckWitness(Isolate* isolate)
8415-
#ifdef V8_ENABLE_DIRECT_LOCAL
8415+
#ifdef V8_ENABLE_DIRECT_HANDLE
84168416
// An empty local suffices.
84178417
: cached_map_()
84188418
#else
@@ -8425,7 +8425,7 @@ v8::TypecheckWitness::TypecheckWitness(Isolate* isolate)
84258425

84268426
void v8::TypecheckWitness::Update(Local<Value> baseline) {
84278427
i::Tagged<i::Object> obj = *Utils::OpenDirectHandle(*baseline);
8428-
#ifdef V8_ENABLE_DIRECT_LOCAL
8428+
#ifdef V8_ENABLE_DIRECT_HANDLE
84298429
if (IsSmi(obj)) {
84308430
cached_map_ = Local<Data>();
84318431
} else {

0 commit comments

Comments
 (0)