From b4d4eb00dffc1bba75ca67be2eec75d345ee834f Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Mon, 24 Feb 2020 20:03:58 +0000 Subject: [PATCH] [vm/gc] Ensure we keep old trace allocations tables around until its safe to delete them (during GC) We do this already for the class table, size table and unboxed field table. This does the same for the trace allocation table. Issue https://github.com/dart-lang/sdk/issues/40749 Change-Id: I417b7daf529967a5a496cc4ecc15cee20371ee9e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136963 Reviewed-by: Ryan Macnak Commit-Queue: Martin Kustermann --- runtime/vm/class_table.cc | 78 ++++++++++++++++----------------------- runtime/vm/class_table.h | 9 +++-- 2 files changed, 37 insertions(+), 50 deletions(-) diff --git a/runtime/vm/class_table.cc b/runtime/vm/class_table.cc index df84820a5a6..583d4ead5d1 100644 --- a/runtime/vm/class_table.cc +++ b/runtime/vm/class_table.cc @@ -22,10 +22,7 @@ DEFINE_FLAG(bool, print_class_table, false, "Print initial class table."); SharedClassTable::SharedClassTable() : top_(kNumPredefinedCids), capacity_(0), - table_(NULL), - old_tables_(new MallocGrowableArray()), - unboxed_fields_map_(nullptr), - old_unboxed_fields_maps_(new MallocGrowableArray()) { + old_tables_(new MallocGrowableArray()) { if (Dart::vm_isolate() == NULL) { ASSERT(kInitialCapacity >= kNumPredefinedCids); capacity_ = kInitialCapacity; @@ -52,33 +49,23 @@ SharedClassTable::SharedClassTable() table_[kNeverCid] = vm_shared_class_table->SizeAt(kNeverCid); } #if defined(SUPPORT_UNBOXED_INSTANCE_FIELDS) + // Note that [calloc] will zero-initialize the memory. unboxed_fields_map_ = static_cast( - malloc(capacity_ * sizeof(UnboxedFieldBitmap))); - memset(unboxed_fields_map_, 0, sizeof(UnboxedFieldBitmap) * capacity_); + calloc(capacity_, sizeof(UnboxedFieldBitmap))); #endif // defined(SUPPORT_UNBOXED_INSTANCE_FIELDS) #ifndef PRODUCT + // Note that [calloc] will zero-initialize the memory. trace_allocation_table_ = - static_cast(malloc(capacity_ * sizeof(uint8_t))); // NOLINT - for (intptr_t i = 0; i < capacity_; i++) { - trace_allocation_table_[i] = 0; - } + static_cast(calloc(capacity_, sizeof(uint8_t))); #endif // !PRODUCT } SharedClassTable::~SharedClassTable() { if (old_tables_ != NULL) { FreeOldTables(); delete old_tables_; - free(table_); - } - - if (old_unboxed_fields_maps_ != nullptr) { - FreeOldUnboxedFieldsMaps(); - delete old_unboxed_fields_maps_; - } - - if (unboxed_fields_map_ != nullptr) { - free(unboxed_fields_map_); } + free(table_); + free(unboxed_fields_map_); NOT_IN_PRODUCT(free(trace_allocation_table_)); } @@ -144,12 +131,6 @@ void SharedClassTable::FreeOldTables() { } } -void SharedClassTable::FreeOldUnboxedFieldsMaps() { - while (old_unboxed_fields_maps_->length() > 0) { - free(old_unboxed_fields_maps_->RemoveLast()); - } -} - void ClassTable::Register(const Class& cls) { ASSERT(Thread::Current()->IsMutatorThread()); @@ -229,11 +210,13 @@ void ClassTable::Grow(intptr_t new_capacity) { auto new_table = static_cast( malloc(new_capacity * sizeof(RawClass*))); // NOLINT - memmove(new_table, table_, top_ * sizeof(RawClass*)); - memset(new_table + top_, 0, (new_capacity - top_) * sizeof(RawClass*)); - capacity_ = new_capacity; + memmove(new_table, table_, capacity_ * sizeof(RawClass*)); + memset(new_table + capacity_, 0, + (new_capacity - capacity_) * sizeof(RawClass*)); old_class_tables_->Add(table_); table_ = new_table; // TODO(koda): This should use atomics. + + capacity_ = new_capacity; } void SharedClassTable::AllocateIndex(intptr_t index) { @@ -257,33 +240,36 @@ void SharedClassTable::Grow(intptr_t new_capacity) { intptr_t* new_table = static_cast( malloc(new_capacity * sizeof(intptr_t))); // NOLINT - memmove(new_table, table_, top_ * sizeof(intptr_t)); - memset(new_table + top_, 0, (new_capacity - top_) * sizeof(intptr_t)); -#ifndef PRODUCT - auto new_stats_table = - static_cast(realloc(trace_allocation_table_, - new_capacity * sizeof(uint8_t))); // NOLINT -#endif - for (intptr_t i = capacity_; i < new_capacity; i++) { - new_table[i] = 0; - NOT_IN_PRODUCT(new_stats_table[i] = 0); - } + memmove(new_table, table_, capacity_ * sizeof(intptr_t)); + memset(new_table + capacity_, 0, + (new_capacity - capacity_) * sizeof(intptr_t)); + +#if !defined(PRODUCT) + auto new_trace_table = + static_cast(malloc(new_capacity * sizeof(uint8_t))); // NOLINT + memmove(new_trace_table, trace_allocation_table_, + capacity_ * sizeof(uint8_t)); + memset(new_trace_table + capacity_, 0, + (new_capacity - capacity_) * sizeof(uint8_t)); +#endif - capacity_ = new_capacity; old_tables_->Add(table_); table_ = new_table; // TODO(koda): This should use atomics. - NOT_IN_PRODUCT(trace_allocation_table_ = new_stats_table); + NOT_IN_PRODUCT(old_tables_->Add(trace_allocation_table_)); + NOT_IN_PRODUCT(trace_allocation_table_ = new_trace_table); #if defined(SUPPORT_UNBOXED_INSTANCE_FIELDS) auto new_unboxed_fields_map = static_cast( malloc(new_capacity * sizeof(UnboxedFieldBitmap))); memmove(new_unboxed_fields_map, unboxed_fields_map_, - top_ * sizeof(UnboxedFieldBitmap)); - memset(new_unboxed_fields_map + top_, 0, - (new_capacity - top_) * sizeof(UnboxedFieldBitmap)); - old_unboxed_fields_maps_->Add(unboxed_fields_map_); + capacity_ * sizeof(UnboxedFieldBitmap)); + memset(new_unboxed_fields_map + capacity_, 0, + (new_capacity - capacity_) * sizeof(UnboxedFieldBitmap)); + old_tables_->Add(unboxed_fields_map_); unboxed_fields_map_ = new_unboxed_fields_map; #endif // defined(SUPPORT_UNBOXED_INSTANCE_FIELDS) + + capacity_ = new_capacity; } void ClassTable::Unregister(intptr_t index) { diff --git a/runtime/vm/class_table.h b/runtime/vm/class_table.h index 990560bea09..0ff29122be0 100644 --- a/runtime/vm/class_table.h +++ b/runtime/vm/class_table.h @@ -203,6 +203,8 @@ class SharedClassTable { static bool ShouldUpdateSizeForClassId(intptr_t cid); #ifndef PRODUCT + // Copy-on-write is used for trace_allocation_table_, with old copies stored + // in old_tables_. uint8_t* trace_allocation_table_ = nullptr; #endif // !PRODUCT @@ -214,8 +216,8 @@ class SharedClassTable { intptr_t capacity_; // Copy-on-write is used for table_, with old copies stored in old_tables_. - intptr_t* table_; // Maps the cid to the instance size. - MallocGrowableArray* old_tables_; + intptr_t* table_ = nullptr; // Maps the cid to the instance size. + MallocGrowableArray* old_tables_; IsolateGroupReloadContext* reload_context_ = nullptr; @@ -224,8 +226,7 @@ class SharedClassTable { // the GC has to scan, a 1 indicates that the word is part of e.g. an unboxed // double and does not need to be scanned. (see Class::Calculate...() where // the bitmap is constructed) - UnboxedFieldBitmap* unboxed_fields_map_; - MallocGrowableArray* old_unboxed_fields_maps_; + UnboxedFieldBitmap* unboxed_fields_map_ = nullptr; DISALLOW_COPY_AND_ASSIGN(SharedClassTable); };