Re-enable adding intern table to image
Changed intern table to have a stack of tables similarily to
ClassTable. Adding an image intern table adds to the front of the
intern table stack. Also some cleanup.
Bug: 26317072
Change-Id: I7bbf9485b5dbbbf3707fed21e29de3beccfb8705
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 7f67ae4..7b32c5b 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -2333,7 +2333,7 @@
if (HasZygoteSpace()) {
return;
}
- Runtime::Current()->GetInternTable()->SwapPostZygoteWithPreZygote();
+ Runtime::Current()->GetInternTable()->AddNewTable();
Runtime::Current()->GetClassLinker()->MoveClassTableToPreZygote();
VLOG(heap) << "Starting PreZygoteFork";
// Trim the pages at the end of the non moving space.
diff --git a/runtime/intern_table.cc b/runtime/intern_table.cc
index d035f5d..68d7ebd 100644
--- a/runtime/intern_table.cc
+++ b/runtime/intern_table.cc
@@ -32,7 +32,8 @@
namespace art {
InternTable::InternTable()
- : image_added_to_intern_table_(false), log_new_roots_(false),
+ : images_added_to_intern_table_(false),
+ log_new_roots_(false),
weak_intern_condition_("New intern condition", *Locks::intern_table_lock_),
weak_root_state_(gc::kWeakRootStateNormal) {
}
@@ -93,10 +94,10 @@
return weak_interns_.Find(s);
}
-void InternTable::SwapPostZygoteWithPreZygote() {
+void InternTable::AddNewTable() {
MutexLock mu(Thread::Current(), *Locks::intern_table_lock_);
- weak_interns_.SwapPostZygoteWithPreZygote();
- strong_interns_.SwapPostZygoteWithPreZygote();
+ weak_interns_.AddNewTable();
+ strong_interns_.AddNewTable();
}
mirror::String* InternTable::InsertStrong(mirror::String* s) {
@@ -153,41 +154,36 @@
void InternTable::AddImageStringsToTable(gc::space::ImageSpace* image_space) {
CHECK(image_space != nullptr);
MutexLock mu(Thread::Current(), *Locks::intern_table_lock_);
- if (!image_added_to_intern_table_) {
- const ImageHeader* const header = &image_space->GetImageHeader();
- // Check if we have the interned strings section.
- const ImageSection& section = header->GetImageSection(ImageHeader::kSectionInternedStrings);
- if (section.Size() > 0) {
- ReadFromMemoryLocked(image_space->Begin() + section.Offset());
- } else {
- // TODO: Delete this logic?
- mirror::Object* root = header->GetImageRoot(ImageHeader::kDexCaches);
- mirror::ObjectArray<mirror::DexCache>* dex_caches = root->AsObjectArray<mirror::DexCache>();
- for (int32_t i = 0; i < dex_caches->GetLength(); ++i) {
- mirror::DexCache* dex_cache = dex_caches->Get(i);
- const size_t num_strings = dex_cache->NumStrings();
- for (size_t j = 0; j < num_strings; ++j) {
- mirror::String* image_string = dex_cache->GetResolvedString(j);
- if (image_string != nullptr) {
- mirror::String* found = LookupStrong(image_string);
- if (found == nullptr) {
- InsertStrong(image_string);
- } else {
- DCHECK_EQ(found, image_string);
- }
+ const ImageHeader* const header = &image_space->GetImageHeader();
+ // Check if we have the interned strings section.
+ const ImageSection& section = header->GetImageSection(ImageHeader::kSectionInternedStrings);
+ if (section.Size() > 0) {
+ AddTableFromMemoryLocked(image_space->Begin() + section.Offset());
+ } else {
+ // TODO: Delete this logic?
+ mirror::Object* root = header->GetImageRoot(ImageHeader::kDexCaches);
+ mirror::ObjectArray<mirror::DexCache>* dex_caches = root->AsObjectArray<mirror::DexCache>();
+ for (int32_t i = 0; i < dex_caches->GetLength(); ++i) {
+ mirror::DexCache* dex_cache = dex_caches->Get(i);
+ const size_t num_strings = dex_cache->NumStrings();
+ for (size_t j = 0; j < num_strings; ++j) {
+ mirror::String* image_string = dex_cache->GetResolvedString(j);
+ if (image_string != nullptr) {
+ mirror::String* found = LookupStrong(image_string);
+ if (found == nullptr) {
+ InsertStrong(image_string);
+ } else {
+ DCHECK_EQ(found, image_string);
}
}
}
}
- image_added_to_intern_table_ = true;
}
+ images_added_to_intern_table_ = true;
}
mirror::String* InternTable::LookupStringFromImage(mirror::String* s) {
- if (image_added_to_intern_table_) {
- return nullptr;
- }
- std::vector<gc::space::ImageSpace*> image_spaces =
+ const std::vector<gc::space::ImageSpace*>& image_spaces =
Runtime::Current()->GetHeap()->GetBootImageSpaces();
if (image_spaces.empty()) {
return nullptr; // No image present.
@@ -284,9 +280,11 @@
return weak;
}
// Check the image for a match.
- mirror::String* image = LookupStringFromImage(s);
- if (image != nullptr) {
- return is_strong ? InsertStrong(image) : InsertWeak(image);
+ if (!images_added_to_intern_table_) {
+ mirror::String* const image_string = LookupStringFromImage(s);
+ if (image_string != nullptr) {
+ return is_strong ? InsertStrong(image_string) : InsertWeak(image_string);
+ }
}
// No match in the strong table or the weak table. Insert into the strong / weak table.
return is_strong ? InsertStrong(s) : InsertWeak(s);
@@ -326,27 +324,18 @@
weak_interns_.SweepWeaks(visitor);
}
-void InternTable::AddImageInternTable(gc::space::ImageSpace* image_space) {
- const ImageSection& intern_section = image_space->GetImageHeader().GetImageSection(
- ImageHeader::kSectionInternedStrings);
- // Read the string tables from the image.
- const uint8_t* ptr = image_space->Begin() + intern_section.Offset();
- const size_t offset = ReadFromMemory(ptr);
- CHECK_LE(offset, intern_section.Size());
-}
-
-size_t InternTable::ReadFromMemory(const uint8_t* ptr) {
+size_t InternTable::AddTableFromMemory(const uint8_t* ptr) {
MutexLock mu(Thread::Current(), *Locks::intern_table_lock_);
- return ReadFromMemoryLocked(ptr);
+ return AddTableFromMemoryLocked(ptr);
}
-size_t InternTable::ReadFromMemoryLocked(const uint8_t* ptr) {
- return strong_interns_.ReadIntoPreZygoteTable(ptr);
+size_t InternTable::AddTableFromMemoryLocked(const uint8_t* ptr) {
+ return strong_interns_.AddTableFromMemory(ptr);
}
size_t InternTable::WriteToMemory(uint8_t* ptr) {
MutexLock mu(Thread::Current(), *Locks::intern_table_lock_);
- return strong_interns_.WriteFromPostZygoteTable(ptr);
+ return strong_interns_.WriteToMemory(ptr);
}
std::size_t InternTable::StringHashEquals::operator()(const GcRoot<mirror::String>& root) const {
@@ -364,71 +353,87 @@
return a.Read()->Equals(b.Read());
}
-size_t InternTable::Table::ReadIntoPreZygoteTable(const uint8_t* ptr) {
- CHECK_EQ(pre_zygote_table_.Size(), 0u);
+size_t InternTable::Table::AddTableFromMemory(const uint8_t* ptr) {
size_t read_count = 0;
- pre_zygote_table_ = UnorderedSet(ptr, false /* make copy */, &read_count);
+ UnorderedSet set(ptr, /*make copy*/false, &read_count);
+ // TODO: Disable this for app images if app images have intern tables.
+ static constexpr bool kCheckDuplicates = true;
+ if (kCheckDuplicates) {
+ for (GcRoot<mirror::String>& string : set) {
+ CHECK(Find(string.Read()) == nullptr) << "Already found " << string.Read()->ToModifiedUtf8();
+ }
+ }
+ // Insert at the front since we insert into the back.
+ tables_.insert(tables_.begin(), std::move(set));
return read_count;
}
-size_t InternTable::Table::WriteFromPostZygoteTable(uint8_t* ptr) {
- return post_zygote_table_.WriteToMemory(ptr);
+size_t InternTable::Table::WriteToMemory(uint8_t* ptr) {
+ if (tables_.empty()) {
+ return 0;
+ }
+ UnorderedSet* table_to_write;
+ UnorderedSet combined;
+ if (tables_.size() > 1) {
+ table_to_write = &combined;
+ for (UnorderedSet& table : tables_) {
+ for (GcRoot<mirror::String>& string : table) {
+ combined.Insert(string);
+ }
+ }
+ } else {
+ table_to_write = &tables_.back();
+ }
+ return table_to_write->WriteToMemory(ptr);
}
void InternTable::Table::Remove(mirror::String* s) {
- auto it = post_zygote_table_.Find(GcRoot<mirror::String>(s));
- if (it != post_zygote_table_.end()) {
- post_zygote_table_.Erase(it);
- } else {
- it = pre_zygote_table_.Find(GcRoot<mirror::String>(s));
- DCHECK(it != pre_zygote_table_.end());
- pre_zygote_table_.Erase(it);
+ for (UnorderedSet& table : tables_) {
+ auto it = table.Find(GcRoot<mirror::String>(s));
+ if (it != table.end()) {
+ table.Erase(it);
+ return;
+ }
}
+ LOG(FATAL) << "Attempting to remove non-interned string " << s->ToModifiedUtf8();
}
mirror::String* InternTable::Table::Find(mirror::String* s) {
Locks::intern_table_lock_->AssertHeld(Thread::Current());
- auto it = pre_zygote_table_.Find(GcRoot<mirror::String>(s));
- if (it != pre_zygote_table_.end()) {
- return it->Read();
- }
- it = post_zygote_table_.Find(GcRoot<mirror::String>(s));
- if (it != post_zygote_table_.end()) {
- return it->Read();
+ for (UnorderedSet& table : tables_) {
+ auto it = table.Find(GcRoot<mirror::String>(s));
+ if (it != table.end()) {
+ return it->Read();
+ }
}
return nullptr;
}
-void InternTable::Table::SwapPostZygoteWithPreZygote() {
- if (pre_zygote_table_.Empty()) {
- std::swap(pre_zygote_table_, post_zygote_table_);
- VLOG(heap) << "Swapping " << pre_zygote_table_.Size() << " interns to the pre zygote table";
- } else {
- // This case happens if read the intern table from the image.
- VLOG(heap) << "Not swapping due to non-empty pre_zygote_table_";
- }
+void InternTable::Table::AddNewTable() {
+ tables_.push_back(UnorderedSet());
}
void InternTable::Table::Insert(mirror::String* s) {
- // Always insert the post zygote table, this gets swapped when we create the zygote to be the
- // pre zygote table.
- post_zygote_table_.Insert(GcRoot<mirror::String>(s));
+ // Always insert the last table, the image tables are before and we avoid inserting into these
+ // to prevent dirty pages.
+ DCHECK(!tables_.empty());
+ tables_.back().Insert(GcRoot<mirror::String>(s));
}
void InternTable::Table::VisitRoots(RootVisitor* visitor) {
BufferedRootVisitor<kDefaultBufferedRootCount> buffered_visitor(
visitor, RootInfo(kRootInternedString));
- for (auto& intern : pre_zygote_table_) {
- buffered_visitor.VisitRoot(intern);
- }
- for (auto& intern : post_zygote_table_) {
- buffered_visitor.VisitRoot(intern);
+ for (UnorderedSet& table : tables_) {
+ for (auto& intern : table) {
+ buffered_visitor.VisitRoot(intern);
+ }
}
}
void InternTable::Table::SweepWeaks(IsMarkedVisitor* visitor) {
- SweepWeaks(&pre_zygote_table_, visitor);
- SweepWeaks(&post_zygote_table_, visitor);
+ for (UnorderedSet& table : tables_) {
+ SweepWeaks(&table, visitor);
+ }
}
void InternTable::Table::SweepWeaks(UnorderedSet* set, IsMarkedVisitor* visitor) {
@@ -446,7 +451,12 @@
}
size_t InternTable::Table::Size() const {
- return pre_zygote_table_.Size() + post_zygote_table_.Size();
+ return std::accumulate(tables_.begin(),
+ tables_.end(),
+ size_t(0),
+ [](size_t sum, const UnorderedSet& set) {
+ return sum + set.Size();
+ });
}
void InternTable::ChangeWeakRootState(gc::WeakRootState new_state) {
@@ -464,10 +474,10 @@
InternTable::Table::Table() {
Runtime* const runtime = Runtime::Current();
- pre_zygote_table_.SetLoadFactor(runtime->GetHashTableMinLoadFactor(),
- runtime->GetHashTableMaxLoadFactor());
- post_zygote_table_.SetLoadFactor(runtime->GetHashTableMinLoadFactor(),
- runtime->GetHashTableMaxLoadFactor());
+ // Initial table.
+ tables_.push_back(UnorderedSet());
+ tables_.back().SetLoadFactor(runtime->GetHashTableMinLoadFactor(),
+ runtime->GetHashTableMaxLoadFactor());
}
} // namespace art
diff --git a/runtime/intern_table.h b/runtime/intern_table.h
index 3a4e8d8..ec0f1e2 100644
--- a/runtime/intern_table.h
+++ b/runtime/intern_table.h
@@ -99,21 +99,20 @@
void BroadcastForNewInterns() SHARED_REQUIRES(Locks::mutator_lock_);
// Adds all of the resolved image strings from the image space into the intern table. The
- // advantage of doing this is preventing expensive DexFile::FindStringId calls.
+ // advantage of doing this is preventing expensive DexFile::FindStringId calls. Sets
+ // images_added_to_intern_table_ to true, AddImageStringsToTable must be called on either all
+ // images or none.
void AddImageStringsToTable(gc::space::ImageSpace* image_space)
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!Locks::intern_table_lock_);
- // Copy the post zygote tables to pre zygote to save memory by preventing dirty pages.
- void SwapPostZygoteWithPreZygote()
- SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!Locks::intern_table_lock_);
-
- // Add an intern table which was serialized to the image.
- void AddImageInternTable(gc::space::ImageSpace* image_space)
+ // Add a new intern table for inserting to, previous intern tables are still there but no
+ // longer inserted into and ideally unmodified. This is done to prevent dirty pages.
+ void AddNewTable()
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!Locks::intern_table_lock_);
// Read the intern table from memory. The elements aren't copied, the intern hash set data will
// point to somewhere within ptr. Only reads the strong interns.
- size_t ReadFromMemory(const uint8_t* ptr) REQUIRES(!Locks::intern_table_lock_)
+ size_t AddTableFromMemory(const uint8_t* ptr) REQUIRES(!Locks::intern_table_lock_)
SHARED_REQUIRES(Locks::mutator_lock_);
// Write the post zygote intern table to a pointer. Only writes the strong interns since it is
@@ -157,15 +156,17 @@
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_);
void SweepWeaks(IsMarkedVisitor* visitor)
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_);
- void SwapPostZygoteWithPreZygote() REQUIRES(Locks::intern_table_lock_);
+ // Add a new intern table that will only be inserted into from now on.
+ void AddNewTable() REQUIRES(Locks::intern_table_lock_);
size_t Size() const REQUIRES(Locks::intern_table_lock_);
- // Read pre zygote table is called from ReadFromMemory which happens during runtime creation
- // when we load the image intern table. Returns how many bytes were read.
- size_t ReadIntoPreZygoteTable(const uint8_t* ptr)
+ // Read and add an intern table from ptr.
+ // Tables read are inserted at the front of the table array. Only checks for conflicts in
+ // debug builds. Returns how many bytes were read.
+ size_t AddTableFromMemory(const uint8_t* ptr)
REQUIRES(Locks::intern_table_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
- // The image writer calls WritePostZygoteTable through WriteToMemory, it writes the interns in
- // the post zygote table. Returns how many bytes were written.
- size_t WriteFromPostZygoteTable(uint8_t* ptr)
+ // Write the intern tables to ptr, if there are multiple tables they are combined into a single one.
+ // Returns how many bytes were written.
+ size_t WriteToMemory(uint8_t* ptr)
REQUIRES(Locks::intern_table_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
private:
@@ -175,12 +176,9 @@
void SweepWeaks(UnorderedSet* set, IsMarkedVisitor* visitor)
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_);
- // We call SwapPostZygoteWithPreZygote when we create the zygote to reduce private dirty pages
- // caused by modifying the zygote intern table hash table. The pre zygote table are the
- // interned strings which were interned before we created the zygote space. Post zygote is self
- // explanatory.
- UnorderedSet pre_zygote_table_;
- UnorderedSet post_zygote_table_;
+ // We call AddNewTable when we create the zygote to reduce private dirty pages caused by
+ // modifying the zygote intern table. The back of table is modified when strings are interned.
+ std::vector<UnorderedSet> tables_;
};
// Insert if non null, otherwise return null. Must be called holding the mutator lock.
@@ -214,7 +212,7 @@
void RemoveWeakFromTransaction(mirror::String* s)
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_);
- size_t ReadFromMemoryLocked(const uint8_t* ptr)
+ size_t AddTableFromMemoryLocked(const uint8_t* ptr)
REQUIRES(Locks::intern_table_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
// Change the weak root state. May broadcast to waiters.
@@ -225,7 +223,7 @@
void WaitUntilAccessible(Thread* self)
REQUIRES(Locks::intern_table_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
- bool image_added_to_intern_table_ GUARDED_BY(Locks::intern_table_lock_);
+ bool images_added_to_intern_table_ GUARDED_BY(Locks::intern_table_lock_);
bool log_new_roots_ GUARDED_BY(Locks::intern_table_lock_);
ConditionVariable weak_intern_condition_ GUARDED_BY(Locks::intern_table_lock_);
// Since this contains (strong) roots, they need a read barrier to
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index dfb2877..01f3bbd 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -543,8 +543,7 @@
// Use !IsAotCompiler so that we get test coverage, tests are never the zygote.
if (!IsAotCompiler()) {
ScopedObjectAccess soa(self);
- std::vector<gc::space::ImageSpace*> image_spaces = heap_->GetBootImageSpaces();
- for (gc::space::ImageSpace* image_space : image_spaces) {
+ for (gc::space::ImageSpace* image_space : heap_->GetBootImageSpaces()) {
ATRACE_BEGIN("AddImageStringsToTable");
GetInternTable()->AddImageStringsToTable(image_space);
ATRACE_END();