From 147f0e3d94c8e6d6e5843b5a7f192bb56fd860a1 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Wed, 6 May 2020 19:19:52 +0000 Subject: [PATCH] [vm] Delay allocating the ObjectIdRing until the first service request or response. Avoids GC overhead when the VM service is not in use. Change-Id: Ic2e752c17fdd01045e30dd62e16f20896a9fc64e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/146440 Reviewed-by: Alexander Aprelev Reviewed-by: Martin Kustermann Commit-Queue: Ryan Macnak --- runtime/vm/heap/become.cc | 5 +++-- runtime/vm/heap/compactor.cc | 5 ++++- runtime/vm/isolate.cc | 17 +++++++++++++---- runtime/vm/isolate.h | 3 ++- runtime/vm/json_stream.cc | 2 +- runtime/vm/object_id_ring.cc | 6 +++--- runtime/vm/object_id_ring.h | 2 +- runtime/vm/object_id_ring_test.cc | 8 ++++---- runtime/vm/service.cc | 3 +-- runtime/vm/service_test.cc | 6 +++--- 10 files changed, 35 insertions(+), 22 deletions(-) diff --git a/runtime/vm/heap/become.cc b/runtime/vm/heap/become.cc index 2799db6302a..76f13030034 100644 --- a/runtime/vm/heap/become.cc +++ b/runtime/vm/heap/become.cc @@ -296,8 +296,9 @@ void Become::FollowForwardingPointers(Thread* thread) { isolate_group->ForEachIsolate( [&](Isolate* isolate) { ObjectIdRing* ring = isolate->object_id_ring(); - ASSERT(ring != NULL); - ring->VisitPointers(&pointer_visitor); + if (ring != nullptr) { + ring->VisitPointers(&pointer_visitor); + } }, /*at_safepoint=*/true); #endif // !PRODUCT diff --git a/runtime/vm/heap/compactor.cc b/runtime/vm/heap/compactor.cc index fbea2f88feb..84dbba5f35c 100644 --- a/runtime/vm/heap/compactor.cc +++ b/runtime/vm/heap/compactor.cc @@ -418,7 +418,10 @@ void CompactorTask::RunEnteredIsolateGroup() { TIMELINE_FUNCTION_GC_DURATION(thread, "ForwardObjectIdRing"); isolate_group_->ForEachIsolate( [&](Isolate* isolate) { - isolate->object_id_ring()->VisitPointers(compactor_); + ObjectIdRing* ring = isolate->object_id_ring(); + if (ring != nullptr) { + ring->VisitPointers(compactor_); + } }, /*at_safepoint=*/true); break; diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index c31b429a4e7..926a2174b1d 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -1475,9 +1475,6 @@ Isolate::Isolate(IsolateGroup* isolate_group, #undef ISOLATE_METRIC_CONSTRUCTORS reload_every_n_stack_overflow_checks_(FLAG_reload_every), #endif // !defined(PRODUCT) -#if !defined(PRODUCT) - object_id_ring_(new ObjectIdRing()), -#endif start_time_micros_(OS::GetCurrentMonotonicMicros()), random_(), mutex_(NOT_IN_PRODUCT("Isolate::mutex_")), @@ -2780,7 +2777,10 @@ void IsolateGroup::VisitStackPointers(ObjectPointerVisitor* visitor, void IsolateGroup::VisitObjectIdRingPointers(ObjectPointerVisitor* visitor) { #if !defined(PRODUCT) for (Isolate* isolate : isolates_) { - isolate->object_id_ring()->VisitPointers(visitor); + ObjectIdRing* ring = isolate->object_id_ring(); + if (ring != nullptr) { + ring->VisitPointers(visitor); + } } #endif // !defined(PRODUCT) } @@ -2840,6 +2840,15 @@ intptr_t IsolateGroup::GetClassSizeForHeapWalkAt(intptr_t cid) { #endif // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME) } +#if !defined(PRODUCT) +ObjectIdRing* Isolate::EnsureObjectIdRing() { + if (object_id_ring_ == nullptr) { + object_id_ring_ = new ObjectIdRing(); + } + return object_id_ring_; +} +#endif // !defined(PRODUCT) + void Isolate::AddPendingDeopt(uword fp, uword pc) { // GrowableArray::Add is not atomic and may be interrupt by a profiler // stack walk. diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 73455aa7773..6852e8bac10 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -955,7 +955,8 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry { } #if !defined(PRODUCT) - ObjectIdRing* object_id_ring() { return object_id_ring_; } + ObjectIdRing* object_id_ring() const { return object_id_ring_; } + ObjectIdRing* EnsureObjectIdRing(); #endif // !defined(PRODUCT) void AddPendingDeopt(uword fp, uword pc); diff --git a/runtime/vm/json_stream.cc b/runtime/vm/json_stream.cc index bae68b30bd5..0fd8b33a8b0 100644 --- a/runtime/vm/json_stream.cc +++ b/runtime/vm/json_stream.cc @@ -41,7 +41,7 @@ JSONStream::JSONStream(intptr_t buf_size) ObjectIdRing* ring = NULL; Isolate* isolate = Isolate::Current(); if (isolate != NULL) { - ring = isolate->object_id_ring(); + ring = isolate->EnsureObjectIdRing(); } default_id_zone_.Init(ring, ObjectIdRing::kAllocateId); } diff --git a/runtime/vm/object_id_ring.cc b/runtime/vm/object_id_ring.cc index 1de2e549151..4e60ce056db 100644 --- a/runtime/vm/object_id_ring.cc +++ b/runtime/vm/object_id_ring.cc @@ -86,16 +86,16 @@ void ObjectIdRing::PrintJSON(JSONStream* js) { } } -ObjectIdRing::ObjectIdRing(int32_t capacity) { - ASSERT(capacity > 0); +ObjectIdRing::ObjectIdRing() { serial_num_ = 0; wrapped_ = false; table_ = NULL; - SetCapacityAndMaxSerial(capacity, kMaxId); + SetCapacityAndMaxSerial(kDefaultCapacity, kMaxId); } void ObjectIdRing::SetCapacityAndMaxSerial(int32_t capacity, int32_t max_serial) { + ASSERT(capacity > 0); ASSERT(max_serial <= kMaxId); capacity_ = capacity; if (table_ != NULL) { diff --git a/runtime/vm/object_id_ring.h b/runtime/vm/object_id_ring.h index 63fe1a74642..deca92ba330 100644 --- a/runtime/vm/object_id_ring.h +++ b/runtime/vm/object_id_ring.h @@ -39,7 +39,7 @@ class ObjectIdRing { static const int32_t kInvalidId = -1; static const int32_t kDefaultCapacity = 8192; - explicit ObjectIdRing(int32_t capacity = kDefaultCapacity); + ObjectIdRing(); ~ObjectIdRing(); // Adds the argument to the ring and returns its id. Note we do not allow diff --git a/runtime/vm/object_id_ring_test.cc b/runtime/vm/object_id_ring_test.cc index 07fa8ea0231..d0d62c96522 100644 --- a/runtime/vm/object_id_ring_test.cc +++ b/runtime/vm/object_id_ring_test.cc @@ -52,7 +52,7 @@ class ObjectIdRingTestHelper { // Test that serial number wrapping works. ISOLATE_UNIT_TEST_CASE(ObjectIdRingSerialWrapTest) { Isolate* isolate = Isolate::Current(); - ObjectIdRing* ring = isolate->object_id_ring(); + ObjectIdRing* ring = isolate->EnsureObjectIdRing(); ObjectIdRingTestHelper::SetCapacityAndMaxSerial(ring, 2, 4); intptr_t id; ObjectIdRing::LookupResult kind = ObjectIdRing::kInvalid; @@ -137,7 +137,7 @@ TEST_CASE(ObjectIdRingScavengeMoveTest) { EXPECT_EQ(3, list_length); Isolate* isolate = thread->isolate(); - ObjectIdRing* ring = isolate->object_id_ring(); + ObjectIdRing* ring = isolate->EnsureObjectIdRing(); ObjectIdRing::LookupResult kind = ObjectIdRing::kInvalid; { @@ -196,7 +196,7 @@ TEST_CASE(ObjectIdRingScavengeMoveTest) { // Test that the ring table is updated with nulls when the old GC collects. ISOLATE_UNIT_TEST_CASE(ObjectIdRingOldGCTest) { Isolate* isolate = thread->isolate(); - ObjectIdRing* ring = isolate->object_id_ring(); + ObjectIdRing* ring = isolate->EnsureObjectIdRing(); ObjectIdRing::LookupResult kind = ObjectIdRing::kInvalid; intptr_t raw_obj_id1 = -1; @@ -241,7 +241,7 @@ ISOLATE_UNIT_TEST_CASE(ObjectIdRingOldGCTest) { // overridden by new entries. ISOLATE_UNIT_TEST_CASE(ObjectIdRingExpiredEntryTest) { Isolate* isolate = Isolate::Current(); - ObjectIdRing* ring = isolate->object_id_ring(); + ObjectIdRing* ring = isolate->EnsureObjectIdRing(); // Insert an object and check we can look it up. String& obj = String::Handle(String::New("I will expire")); diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc index fafe51daf7f..62abc18f927 100644 --- a/runtime/vm/service.cc +++ b/runtime/vm/service.cc @@ -1685,8 +1685,7 @@ static ObjectPtr LookupObjectId(Thread* thread, return Object::null(); } - ObjectIdRing* ring = thread->isolate()->object_id_ring(); - ASSERT(ring != NULL); + ObjectIdRing* ring = thread->isolate()->EnsureObjectIdRing(); intptr_t id = -1; if (!GetIntegerId(arg, &id)) { *kind = ObjectIdRing::kInvalid; diff --git a/runtime/vm/service_test.cc b/runtime/vm/service_test.cc index 11fb555dff0..152765e7920 100644 --- a/runtime/vm/service_test.cc +++ b/runtime/vm/service_test.cc @@ -186,7 +186,7 @@ ISOLATE_UNIT_TEST_CASE(Service_IsolateStickyError) { ISOLATE_UNIT_TEST_CASE(Service_IdZones) { Zone* zone = thread->zone(); Isolate* isolate = thread->isolate(); - ObjectIdRing* ring = isolate->object_id_ring(); + ObjectIdRing* ring = isolate->EnsureObjectIdRing(); const String& test_a = String::Handle(zone, String::New("a")); const String& test_b = String::Handle(zone, String::New("b")); @@ -384,7 +384,7 @@ ISOLATE_UNIT_TEST_CASE(Service_PcDescriptors) { const PcDescriptors& descriptors = PcDescriptors::Handle(code_c.pc_descriptors()); EXPECT(!descriptors.IsNull()); - ObjectIdRing* ring = isolate->object_id_ring(); + ObjectIdRing* ring = isolate->EnsureObjectIdRing(); intptr_t id = ring->GetIdForObject(descriptors.raw()); // Build a mock message handler and wrap it in a dart port. @@ -455,7 +455,7 @@ ISOLATE_UNIT_TEST_CASE(Service_LocalVarDescriptors) { const LocalVarDescriptors& descriptors = LocalVarDescriptors::Handle(code_c.GetLocalVarDescriptors()); // Generate an ID for this object. - ObjectIdRing* ring = isolate->object_id_ring(); + ObjectIdRing* ring = isolate->EnsureObjectIdRing(); intptr_t id = ring->GetIdForObject(descriptors.raw()); // Build a mock message handler and wrap it in a dart port.