Skip to content

Commit dbbaccc

Browse files
sygCommit Bot
authored andcommitted
[weakrefs] Port FinalizationRegistry cleanup loop to Torque
To avoid shrinking the unregister token map on each pop of the cleared cell list, the Torque implementation of the cleanup loop avoids shrinking the map until the end of the loop. To support that, PopClearedCellHoldings is refactored to the Torque PopClearedCell which calls the JSFinalization::RemoveCellFromUnregisterTokenMap and the runtime ShrinkFinalizationRegistryUnregisterTokenMap. The former cannot GC is and is implemented in CSA as a fast C call. The latter can GC and is a runtime call. This also incidentally makes uses of FinalizationRegistry without unregister token a fast path that doesn't have to leave Torque. Bug: v8:8179 Change-Id: Ia0c3c5800d26e31319a818f164f6bd3267355aa6 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2137950 Commit-Queue: Shu-yu Guo <syg@chromium.org> Reviewed-by: Marja Hölttä <marja@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#67161}
1 parent 4821ca2 commit dbbaccc

19 files changed

Lines changed: 281 additions & 154 deletions

BUILD.gn

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,7 @@ torque_files = [
10361036
"src/builtins/convert.tq",
10371037
"src/builtins/console.tq",
10381038
"src/builtins/data-view.tq",
1039+
"src/builtins/finalization-registry.tq",
10391040
"src/builtins/frames.tq",
10401041
"src/builtins/frame-arguments.tq",
10411042
"src/builtins/growable-fixed-array.tq",
@@ -2915,6 +2916,7 @@ v8_source_set("v8_base_without_compiler") {
29152916
"src/runtime/runtime-typedarray.cc",
29162917
"src/runtime/runtime-utils.h",
29172918
"src/runtime/runtime-wasm.cc",
2919+
"src/runtime/runtime-weak-refs.cc",
29182920
"src/runtime/runtime.cc",
29192921
"src/runtime/runtime.h",
29202922
"src/sanitizer/asan.h",

src/api/api.cc

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8429,9 +8429,13 @@ Maybe<bool> FinalizationGroup::Cleanup(
84298429
ENTER_V8(isolate, context, FinalizationGroup, Cleanup, Nothing<bool>(),
84308430
i::HandleScope);
84318431
i::Handle<i::Object> callback(fr->cleanup(), isolate);
8432+
i::Handle<i::Object> argv[] = {callback};
84328433
fr->set_scheduled_for_cleanup(false);
84338434
has_pending_exception =
8434-
i::JSFinalizationRegistry::Cleanup(isolate, fr, callback).IsNothing();
8435+
i::Execution::CallBuiltin(isolate,
8436+
isolate->finalization_registry_cleanup_some(),
8437+
fr, arraysize(argv), argv)
8438+
.is_null();
84358439
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
84368440
return Just(true);
84378441
}
@@ -11245,8 +11249,11 @@ void InvokeFinalizationRegistryCleanupFromTask(
1124511249
Local<v8::Context> api_context = Utils::ToLocal(context);
1124611250
CallDepthScope<true> call_depth_scope(isolate, api_context);
1124711251
VMState<OTHER> state(isolate);
11248-
if (JSFinalizationRegistry::Cleanup(isolate, finalization_registry, callback)
11249-
.IsNothing()) {
11252+
Handle<Object> argv[] = {callback};
11253+
if (Execution::CallBuiltin(isolate,
11254+
isolate->finalization_registry_cleanup_some(),
11255+
finalization_registry, arraysize(argv), argv)
11256+
.is_null()) {
1125011257
call_depth_scope.Escape();
1125111258
}
1125211259
}

src/builtins/base.tq

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ extern enum MessageTemplate {
303303
kProxyGetPrototypeOfNonExtensible,
304304
kProxySetPrototypeOfNonExtensible,
305305
kProxyDeletePropertyNonExtensible,
306+
kWeakRefsCleanupMustBeCallable,
306307
...
307308
}
308309

@@ -701,7 +702,8 @@ macro Float64IsNaN(n: float64): bool {
701702
}
702703

703704
// The type of all tagged values that can safely be compared with TaggedEqual.
704-
type TaggedWithIdentity = JSReceiver|FixedArrayBase|Oddball|Map|EmptyString;
705+
type TaggedWithIdentity =
706+
JSReceiver|FixedArrayBase|Oddball|Map|WeakCell|EmptyString;
705707

706708
extern operator '==' macro TaggedEqual(TaggedWithIdentity, Object): bool;
707709
extern operator '==' macro TaggedEqual(Object, TaggedWithIdentity): bool;

src/builtins/builtins-definitions.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,6 @@ namespace internal {
967967
CPP(Trace) \
968968
\
969969
/* Weak refs */ \
970-
CPP(FinalizationRegistryCleanupSome) \
971970
CPP(FinalizationRegistryConstructor) \
972971
CPP(FinalizationRegistryRegister) \
973972
CPP(FinalizationRegistryUnregister) \

src/builtins/builtins-weak-refs.cc

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -122,43 +122,6 @@ BUILTIN(FinalizationRegistryUnregister) {
122122
return *isolate->factory()->ToBoolean(success);
123123
}
124124

125-
BUILTIN(FinalizationRegistryCleanupSome) {
126-
HandleScope scope(isolate);
127-
const char* method_name = "FinalizationRegistry.prototype.cleanupSome";
128-
129-
// 1. Let finalizationGroup be the this value.
130-
//
131-
// 2. If Type(finalizationGroup) is not Object, throw a TypeError
132-
// exception.
133-
//
134-
// 3. If finalizationGroup does not have a [[Cells]] internal slot,
135-
// throw a TypeError exception.
136-
CHECK_RECEIVER(JSFinalizationRegistry, finalization_registry, method_name);
137-
138-
Handle<Object> callback(finalization_registry->cleanup(), isolate);
139-
Handle<Object> callback_obj = args.atOrUndefined(isolate, 1);
140-
141-
// 4. If callback is not undefined and IsCallable(callback) is
142-
// false, throw a TypeError exception.
143-
if (!callback_obj->IsUndefined(isolate)) {
144-
if (!callback_obj->IsCallable()) {
145-
THROW_NEW_ERROR_RETURN_FAILURE(
146-
isolate,
147-
NewTypeError(MessageTemplate::kWeakRefsCleanupMustBeCallable));
148-
}
149-
callback = callback_obj;
150-
}
151-
152-
// Don't do set_scheduled_for_cleanup(false); we still have the task
153-
// scheduled.
154-
if (JSFinalizationRegistry::Cleanup(isolate, finalization_registry, callback)
155-
.IsNothing()) {
156-
DCHECK(isolate->has_pending_exception());
157-
return ReadOnlyRoots(isolate).exception();
158-
}
159-
return ReadOnlyRoots(isolate).undefined_value();
160-
}
161-
162125
BUILTIN(WeakRefConstructor) {
163126
HandleScope scope(isolate);
164127
Handle<JSFunction> target = args.target();
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright 2020 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
namespace runtime {
6+
extern runtime
7+
ShrinkFinalizationRegistryUnregisterTokenMap(Context, JSFinalizationRegistry):
8+
void;
9+
}
10+
11+
namespace weakref {
12+
extern transitioning macro
13+
RemoveFinalizationRegistryCellFromUnregisterTokenMap(
14+
JSFinalizationRegistry, WeakCell): void;
15+
16+
macro SplitOffTail(weakCell: WeakCell): WeakCell|Undefined {
17+
const weakCellTail = weakCell.next;
18+
weakCell.next = Undefined;
19+
typeswitch (weakCellTail) {
20+
case (Undefined): {
21+
}
22+
case (tailIsNowAHead: WeakCell): {
23+
assert(tailIsNowAHead.prev == weakCell);
24+
tailIsNowAHead.prev = Undefined;
25+
}
26+
}
27+
return weakCellTail;
28+
}
29+
30+
transitioning macro
31+
PopClearedCell(finalizationRegistry: JSFinalizationRegistry): WeakCell
32+
|Undefined {
33+
typeswitch (finalizationRegistry.cleared_cells) {
34+
case (Undefined): {
35+
return Undefined;
36+
}
37+
case (weakCell: WeakCell): {
38+
assert(weakCell.prev == Undefined);
39+
finalizationRegistry.cleared_cells = SplitOffTail(weakCell);
40+
41+
// If the WeakCell has an unregister token, remove the cell from the
42+
// unregister token linked lists and and the unregister token from
43+
// key_map. This doesn't shrink key_map, which is done manually after
44+
// the cleanup loop to avoid a runtime call.
45+
if (weakCell.unregister_token != Undefined) {
46+
RemoveFinalizationRegistryCellFromUnregisterTokenMap(
47+
finalizationRegistry, weakCell);
48+
}
49+
50+
return weakCell;
51+
}
52+
}
53+
}
54+
55+
transitioning macro
56+
FinalizationRegistryCleanupLoop(implicit context: Context)(
57+
finalizationRegistry: JSFinalizationRegistry, callback: Callable) {
58+
while (true) {
59+
const weakCellHead = PopClearedCell(finalizationRegistry);
60+
typeswitch (weakCellHead) {
61+
case (Undefined): {
62+
break;
63+
}
64+
case (weakCell: WeakCell): {
65+
try {
66+
Call(context, callback, Undefined, weakCell.holdings);
67+
} catch (e) {
68+
runtime::ShrinkFinalizationRegistryUnregisterTokenMap(
69+
context, finalizationRegistry);
70+
ReThrow(context, e);
71+
}
72+
}
73+
}
74+
}
75+
76+
runtime::ShrinkFinalizationRegistryUnregisterTokenMap(
77+
context, finalizationRegistry);
78+
}
79+
80+
transitioning javascript builtin
81+
FinalizationRegistryPrototypeCleanupSome(
82+
js-implicit context: NativeContext,
83+
receiver: JSAny)(...arguments): JSAny {
84+
// 1. Let finalizationRegistry be the this value.
85+
//
86+
// 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]).
87+
const methodName: constexpr string =
88+
'FinalizationRegistry.prototype.cleanupSome';
89+
const finalizationRegistry =
90+
Cast<JSFinalizationRegistry>(receiver) otherwise ThrowTypeError(
91+
MessageTemplate::kIncompatibleMethodReceiver, methodName, receiver);
92+
93+
let callback: Callable;
94+
if (arguments[0] != Undefined) {
95+
// 4. If callback is not undefined and IsCallable(callback) is
96+
// false, throw a TypeError exception.
97+
callback = Cast<Callable>(arguments[0]) otherwise ThrowTypeError(
98+
MessageTemplate::kWeakRefsCleanupMustBeCallable, arguments[0]);
99+
} else {
100+
callback = finalizationRegistry.cleanup;
101+
}
102+
103+
FinalizationRegistryCleanupLoop(finalizationRegistry, callback);
104+
return Undefined;
105+
}
106+
}

src/codegen/code-stub-assembler.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13250,6 +13250,21 @@ TNode<String> CodeStubAssembler::TaggedToDirectString(TNode<Object> value,
1325013250
return CAST(value);
1325113251
}
1325213252

13253+
void CodeStubAssembler::RemoveFinalizationRegistryCellFromUnregisterTokenMap(
13254+
TNode<JSFinalizationRegistry> finalization_registry,
13255+
TNode<WeakCell> weak_cell) {
13256+
const TNode<ExternalReference> remove_cell = ExternalConstant(
13257+
ExternalReference::
13258+
js_finalization_registry_remove_cell_from_unregister_token_map());
13259+
const TNode<ExternalReference> isolate_ptr =
13260+
ExternalConstant(ExternalReference::isolate_address(isolate()));
13261+
13262+
CallCFunction(remove_cell, MachineType::Pointer(),
13263+
std::make_pair(MachineType::Pointer(), isolate_ptr),
13264+
std::make_pair(MachineType::AnyTagged(), finalization_registry),
13265+
std::make_pair(MachineType::AnyTagged(), weak_cell));
13266+
}
13267+
1325313268
PrototypeCheckAssembler::PrototypeCheckAssembler(
1325413269
compiler::CodeAssemblerState* state, Flags flags,
1325513270
TNode<NativeContext> native_context, TNode<Map> initial_prototype_map,

src/codegen/code-stub-assembler.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3842,6 +3842,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
38423842

38433843
TNode<Smi> RefillMathRandom(TNode<NativeContext> native_context);
38443844

3845+
void RemoveFinalizationRegistryCellFromUnregisterTokenMap(
3846+
TNode<JSFinalizationRegistry> finalization_registry,
3847+
TNode<WeakCell> weak_cell);
3848+
38453849
private:
38463850
friend class CodeStubArguments;
38473851

src/codegen/external-reference.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,10 @@ static int EnterMicrotaskContextWrapper(HandleScopeImplementer* hsi,
902902

903903
FUNCTION_REFERENCE(call_enter_context_function, EnterMicrotaskContextWrapper)
904904

905+
FUNCTION_REFERENCE(
906+
js_finalization_registry_remove_cell_from_unregister_token_map,
907+
JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap)
908+
905909
bool operator==(ExternalReference lhs, ExternalReference rhs) {
906910
return lhs.address() == rhs.address();
907911
}

src/codegen/external-reference.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,8 @@ class StatsCounter;
215215
V(atomic_pair_exchange_function, "atomic_pair_exchange_function") \
216216
V(atomic_pair_compare_exchange_function, \
217217
"atomic_pair_compare_exchange_function") \
218+
V(js_finalization_registry_remove_cell_from_unregister_token_map, \
219+
"JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap") \
218220
EXTERNAL_REFERENCE_LIST_INTL(V)
219221

220222
#ifdef V8_INTL_SUPPORT

0 commit comments

Comments
 (0)