GeezerVison is showing github.com — src: remove dependency on wrapper-descriptor-based CppHeap · nodejs/node@0be79f4
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: src/env-inl.h -25Lines changed: 0 additions & 25 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -62,31 +62,6 @@ inline uv_loop_t* IsolateData::event_loop() const { | 62 | 62 | return event_loop_; | 63 | 63 | } | 64 | 64 |
| 65 | | -inline void IsolateData::SetCppgcReference(v8::Isolate* isolate, | 66 | | - v8::Local<v8::Object> object, | 67 | | - void* wrappable) { | 68 | | - v8::CppHeap* heap = isolate->GetCppHeap(); | 69 | | - CHECK_NOT_NULL(heap); | 70 | | - v8::WrapperDescriptor descriptor = heap->wrapper_descriptor(); | 71 | | - uint16_t required_size = std::max(descriptor.wrappable_instance_index, | 72 | | - descriptor.wrappable_type_index); | 73 | | - CHECK_GT(object->InternalFieldCount(), required_size); | 74 | | - | 75 | | - uint16_t* id_ptr = nullptr; | 76 | | - { | 77 | | - Mutex::ScopedLock lock(isolate_data_mutex_); | 78 | | - auto it = | 79 | | - wrapper_data_map_.find(descriptor.embedder_id_for_garbage_collected); | 80 | | - CHECK_NE(it, wrapper_data_map_.end()); | 81 | | - id_ptr = &(it->second->cppgc_id); | 82 | | - } | 83 | | - | 84 | | - object->SetAlignedPointerInInternalField(descriptor.wrappable_type_index, | 85 | | - id_ptr); | 86 | | - object->SetAlignedPointerInInternalField(descriptor.wrappable_instance_index, | 87 | | - wrappable); | 88 | | -} | 89 | | - | 90 | 65 | inline uint16_t* IsolateData::embedder_id_for_cppgc() const { | 91 | 66 | return &(wrapper_data_->cppgc_id); | 92 | 67 | } | |
Collapse file Copy file name to clipboardExpand all lines: src/env.cc +19-30Lines changed: 19 additions & 30 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 23 | 23 | #include "util-inl.h" | 24 | 24 | #include "v8-cppgc.h" | 25 | 25 | #include "v8-profiler.h" | | 26 | +#include "v8-sandbox.h" // v8::Object::Wrap(), v8::Object::Unwrap() | 26 | 27 |
| 27 | 28 | #include <algorithm> | 28 | 29 | #include <atomic> | @@ -71,7 +72,6 @@ using v8::TryCatch; | 71 | 72 | using v8::Uint32; | 72 | 73 | using v8::Undefined; | 73 | 74 | using v8::Value; | 74 | | -using v8::WrapperDescriptor; | 75 | 75 | using worker::Worker; | 76 | 76 |
| 77 | 77 | int const ContextEmbedderTag::kNodeContextTag = 0x6e6f64; | @@ -533,6 +533,14 @@ void IsolateData::CreateProperties() { | 533 | 533 | CreateEnvProxyTemplate(this); | 534 | 534 | } | 535 | 535 |
| | 536 | +// Previously, the general convention of the wrappable layout for cppgc in | | 537 | +// the ecosystem is: | | 538 | +// [ 0 ] -> embedder id | | 539 | +// [ 1 ] -> wrappable instance | | 540 | +// Now V8 has deprecated this layout-based tracing enablement, embedders | | 541 | +// should simply use v8::Object::Wrap() and v8::Object::Unwrap(). We preserve | | 542 | +// this layout only to distinguish internally how the memory of a Node.js | | 543 | +// wrapper is managed or whether a wrapper is managed by Node.js. | 536 | 544 | constexpr uint16_t kDefaultCppGCEmbedderID = 0x90de; | 537 | 545 | Mutex IsolateData::isolate_data_mutex_; | 538 | 546 | std::unordered_map< uint16_t, std::unique_ptr<PerIsolateWrapperData>> | @@ -570,36 +578,16 @@ IsolateData::IsolateData(Isolate* isolate, | 570 | 578 | v8::CppHeap* cpp_heap = isolate->GetCppHeap(); | 571 | 579 |
| 572 | 580 | uint16_t cppgc_id = kDefaultCppGCEmbedderID; | 573 | | - if (cpp_heap != nullptr) { | 574 | | - // The general convention of the wrappable layout for cppgc in the | 575 | | - // ecosystem is: | 576 | | - // [ 0 ] -> embedder id | 577 | | - // [ 1 ] -> wrappable instance | 578 | | - // If the Isolate includes a CppHeap attached by another embedder, | 579 | | - // And if they also use the field 0 for the ID, we DCHECK that | 580 | | - // the layout matches our layout, and record the embedder ID for cppgc | 581 | | - // to avoid accidentally enabling cppgc on non-cppgc-managed wrappers . | 582 | | - v8::WrapperDescriptor descriptor = cpp_heap->wrapper_descriptor(); | 583 | | - if (descriptor.wrappable_type_index == BaseObject::kEmbedderType) { | 584 | | - cppgc_id = descriptor.embedder_id_for_garbage_collected; | 585 | | - DCHECK_EQ(descriptor.wrappable_instance_index, BaseObject::kSlot); | 586 | | - } | 587 | | - // If the CppHeap uses the slot we use to put non-cppgc-traced BaseObject | 588 | | - // for embedder ID, V8 could accidentally enable cppgc on them. So | 589 | | - // safe guard against this. | 590 | | - DCHECK_NE(descriptor.wrappable_type_index, BaseObject::kSlot); | 591 | | - } else { | 592 | | - cpp_heap_ = CppHeap::Create( | 593 | | - platform, | 594 | | - CppHeapCreateParams{ | 595 | | - {}, | 596 | | - WrapperDescriptor( | 597 | | - BaseObject::kEmbedderType, BaseObject::kSlot, cppgc_id)}); | 598 | | - isolate->AttachCppHeap(cpp_heap_.get()); | 599 | | - } | 600 | 581 | // We do not care about overflow since we just want this to be different | 601 | 582 | // from the cppgc id. | 602 | 583 | uint16_t non_cppgc_id = cppgc_id + 1; | | 584 | + if (cpp_heap == nullptr) { | | 585 | + cpp_heap_ = CppHeap::Create(platform, v8::CppHeapCreateParams{{}}); | | 586 | + // TODO(joyeecheung): pass it into v8::Isolate::CreateParams and let V8 | | 587 | + // own it when we can keep the isolate registered/task runner discoverable | | 588 | + // during isolate disposal. | | 589 | + isolate->AttachCppHeap(cpp_heap_.get()); | | 590 | + } | 603 | 591 |
| 604 | 592 | { | 605 | 593 | // GC could still be run after the IsolateData is destroyed, so we store | @@ -631,11 +619,12 @@ IsolateData::~IsolateData() { | 631 | 619 | } | 632 | 620 | } | 633 | 621 |
| 634 | | -// Public API | | 622 | +// Deprecated API, embedders should use v8::Object::Wrap() directly instead. | 635 | 623 | void SetCppgcReference(Isolate* isolate, | 636 | 624 | Local<Object> object, | 637 | 625 | void* wrappable) { | 638 | | - IsolateData::SetCppgcReference(isolate, object, wrappable); | | 626 | + v8::Object::Wrap<v8::CppHeapPointerTag::kDefaultTag>( | | 627 | + isolate, object, wrappable); | 639 | 628 | } | 640 | 629 |
| 641 | 630 | void IsolateData::MemoryInfo(MemoryTracker* tracker) const { | |
Collapse file Copy file name to clipboardExpand all lines: src/env.h -4Lines changed: 0 additions & 4 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -175,10 +175,6 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { | 175 | 175 | uint16_t* embedder_id_for_cppgc() const; | 176 | 176 | uint16_t* embedder_id_for_non_cppgc() const; | 177 | 177 |
| 178 | | - static inline void SetCppgcReference(v8::Isolate* isolate, | 179 | | - v8::Local<v8::Object> object, | 180 | | - void* wrappable); | 181 | | - | 182 | 178 | inline uv_loop_t* event_loop() const; | 183 | 179 | inline MultiIsolatePlatform* platform() const; | 184 | 180 | inline const SnapshotData* snapshot_data() const; | |
Collapse file Copy file name to clipboardExpand all lines: src/node.h +8-18Lines changed: 8 additions & 18 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1552,24 +1552,14 @@ void RegisterSignalHandler(int signal, | 1552 | 1552 | bool reset_handler = false); | 1553 | 1553 | #endif // _WIN32 | 1554 | 1554 |
| 1555 | | -// Configure the layout of the JavaScript object with a cppgc::GarbageCollected | 1556 | | -// instance so that when the JavaScript object is reachable, the garbage | 1557 | | -// collected instance would have its Trace() method invoked per the cppgc | 1558 | | -// contract. To make it work, the process must have called | 1559 | | -// cppgc::InitializeProcess() before, which is usually the case for addons | 1560 | | -// loaded by the stand-alone Node.js executable. Embedders of Node.js can use | 1561 | | -// either need to call it themselves or make sure that | 1562 | | -// ProcessInitializationFlags::kNoInitializeCppgc is *not* set for cppgc to | 1563 | | -// work. | 1564 | | -// If the CppHeap is owned by Node.js, which is usually the case for addon, | 1565 | | -// the object must be created with at least two internal fields available, | 1566 | | -// and the first two internal fields would be configured by Node.js. | 1567 | | -// This may be superseded by a V8 API in the future, see | 1568 | | -// https://bugs.chromium.org/p/v8/issues/detail?id=13960. Until then this | 1569 | | -// serves as a helper for Node.js isolates. | 1570 | | -NODE_EXTERN void SetCppgcReference(v8::Isolate* isolate, | 1571 | | - v8::Local<v8::Object> object, | 1572 | | - void* wrappable); | | 1555 | +// This is kept as a compatibility layer for addons to wrap cppgc-managed | | 1556 | +// objects on Node.js versions without v8::Object::Wrap(). Addons created to | | 1557 | +// work with only Node.js versions with v8::Object::Wrap() should use that | | 1558 | +// instead. | | 1559 | +NODE_DEPRECATED("Use v8::Object::Wrap()", | | 1560 | + NODE_EXTERN void SetCppgcReference(v8::Isolate* isolate, | | 1561 | + v8::Local<v8::Object> object, | | 1562 | + void* wrappable)); | 1573 | 1563 |
| 1574 | 1564 | } // namespace node | 1575 | 1565 |
| |
Collapse file Copy file name to clipboardExpand all lines: test/addons/cppgc-object/binding.cc +6-8Lines changed: 6 additions & 8 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +#include <assert.h> | 1 | 2 | #include <cppgc/allocation.h> | 2 | 3 | #include <cppgc/garbage-collected.h> | 3 | 4 | #include <cppgc/heap.h> | 4 | 5 | #include <node.h> | 5 | 6 | #include <v8-cppgc.h> | | 7 | +#include <v8-sandbox.h> | 6 | 8 | #include <v8.h> | 7 | 9 | #include <algorithm> | 8 | 10 |
| @@ -15,21 +17,17 @@ class CppGCed : public cppgc::GarbageCollected<CppGCed> { | 15 | 17 | static void New(const v8::FunctionCallbackInfo<v8::Value>& args) { | 16 | 18 | v8::Isolate* isolate = args.GetIsolate(); | 17 | 19 | v8::Local<v8::Object> js_object = args.This(); | 18 | | - CppGCed* gc_object = cppgc::MakeGarbageCollected<CppGCed>( | 19 | | - isolate->GetCppHeap()->GetAllocationHandle()); | | 20 | + auto* heap = isolate->GetCppHeap(); | | 21 | + assert(heap != nullptr); | | 22 | + CppGCed* gc_object = | | 23 | + cppgc::MakeGarbageCollected<CppGCed>(heap->GetAllocationHandle()); | 20 | 24 | node::SetCppgcReference(isolate, js_object, gc_object); | 21 | 25 | args.GetReturnValue().Set(js_object); | 22 | 26 | } | 23 | 27 |
| 24 | 28 | static v8::Local<v8::Function> GetConstructor( | 25 | 29 | v8::Local<v8::Context> context) { | 26 | 30 | auto ft = v8::FunctionTemplate::New(context->GetIsolate(), New); | 27 | | - auto ot = ft->InstanceTemplate(); | 28 | | - v8::WrapperDescriptor descriptor = | 29 | | - context->GetIsolate()->GetCppHeap()->wrapper_descriptor(); | 30 | | - uint16_t required_size = std::max(descriptor.wrappable_instance_index, | 31 | | - descriptor.wrappable_type_index); | 32 | | - ot->SetInternalFieldCount(required_size + 1); | 33 | 31 | return ft->GetFunction(context).ToLocalChecked(); | 34 | 32 | } | 35 | 33 |
| |
Collapse file Copy file name to clipboardExpand all lines: test/cctest/test_cppgc.cc +12-19Lines changed: 12 additions & 19 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 3 | 3 | #include <cppgc/heap.h> | 4 | 4 | #include <node.h> | 5 | 5 | #include <v8-cppgc.h> | | 6 | +#include <v8-sandbox.h> | 6 | 7 | #include <v8.h> | 7 | 8 | #include "node_test_fixture.h" | 8 | 9 |
| 9 | 10 | // This tests that Node.js can work with an existing CppHeap. | 10 | 11 |
| 11 | | -// Mimic the Blink layout. | 12 | | -static int kWrappableTypeIndex = 0; | 13 | | -static int kWrappableInstanceIndex = 1; | 14 | | -static uint16_t kEmbedderID = 0x1; | 15 | | - | 16 | 12 | // Mimic a class that does not know about Node.js. | 17 | 13 | class CppGCed : public cppgc::GarbageCollected<CppGCed> { | 18 | 14 | public: | @@ -23,21 +19,18 @@ class CppGCed : public cppgc::GarbageCollected<CppGCed> { | 23 | 19 | static void New(const v8::FunctionCallbackInfo<v8::Value>& args) { | 24 | 20 | v8::Isolate* isolate = args.GetIsolate(); | 25 | 21 | v8::Local<v8::Object> js_object = args.This(); | 26 | | - CppGCed* gc_object = cppgc::MakeGarbageCollected<CppGCed>( | 27 | | - isolate->GetCppHeap()->GetAllocationHandle()); | 28 | | - js_object->SetAlignedPointerInInternalField(kWrappableTypeIndex, | 29 | | - &kEmbedderID); | 30 | | - js_object->SetAlignedPointerInInternalField(kWrappableInstanceIndex, | 31 | | - gc_object); | | 22 | + auto* heap = isolate->GetCppHeap(); | | 23 | + CHECK_NOT_NULL(heap); | | 24 | + CppGCed* gc_object = | | 25 | + cppgc::MakeGarbageCollected<CppGCed>(heap->GetAllocationHandle()); | | 26 | + node::SetCppgcReference(isolate, js_object, gc_object); | 32 | 27 | kConstructCount++; | 33 | 28 | args.GetReturnValue().Set(js_object); | 34 | 29 | } | 35 | 30 |
| 36 | 31 | static v8::Local<v8::Function> GetConstructor( | 37 | 32 | v8::Local<v8::Context> context) { | 38 | 33 | auto ft = v8::FunctionTemplate::New(context->GetIsolate(), New); | 39 | | - auto ot = ft->InstanceTemplate(); | 40 | | - ot->SetInternalFieldCount(2); | 41 | 34 | return ft->GetFunction(context).ToLocalChecked(); | 42 | 35 | } | 43 | 36 |
| @@ -58,12 +51,12 @@ TEST_F(NodeZeroIsolateTestFixture, ExistingCppHeapTest) { | 58 | 51 |
| 59 | 52 | // Create and attach the CppHeap before we set up the IsolateData so that | 60 | 53 | // it recognizes the existing heap. | 61 | | - std::unique_ptr<v8::CppHeap> cpp_heap = v8::CppHeap::Create( | 62 | | - platform.get(), | 63 | | - v8::CppHeapCreateParams( | 64 | | - {}, | 65 | | - v8::WrapperDescriptor( | 66 | | - kWrappableTypeIndex, kWrappableInstanceIndex, kEmbedderID))); | | 54 | + std::unique_ptr<v8::CppHeap> cpp_heap = | | 55 | + v8::CppHeap::Create(platform.get(), v8::CppHeapCreateParams{{}}); | | 56 | + | | 57 | + // TODO(joyeecheung): pass it into v8::Isolate::CreateParams and let V8 | | 58 | + // own it when we can keep the isolate registered/task runner discoverable | | 59 | + // during isolate disposal. | 67 | 60 | isolate->AttachCppHeap(cpp_heap.get()); | 68 | 61 |
| 69 | 62 | // Try creating Context + IsolateData + Environment. | |
|