From 590fee9e8972f872301c2d16a575d579ee564bee Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Fri, 13 Sep 2013 13:46:47 -0700 Subject: Compacting collector. The compacting collector is currently similar to semispace. It works by copying objects back and forth between two bump pointer spaces. There are types of objects which are "non-movable" due to current runtime limitations. These are Classes, Methods, and Fields. Bump pointer spaces are a new type of continuous alloc space which have no lock in the allocation code path. When you allocate from these it uses atomic operations to increase an index. Traversing the objects in the bump pointer space relies on Object::SizeOf matching the allocated size exactly. Runtime changes: JNI::GetArrayElements returns copies objects if you attempt to get the backing data of a movable array. For GetArrayElementsCritical, we return direct backing storage for any types of arrays, but temporarily disable the GC until the critical region is completed. Added a new runtime call called VisitObjects, this is used in place of the old pattern which was flushing the allocation stack and walking the bitmaps. Changed image writer to be compaction safe and use object monitor word for forwarding addresses. Added a bunch of added SIRTs to ClassLinker, MethodLinker, etc.. TODO: Enable switching allocators, compacting on background, etc.. Bug: 8981901 Change-Id: I3c886fd322a6eef2b99388d19a765042ec26ab99 --- compiler/driver/compiler_driver.cc | 97 +++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 49 deletions(-) (limited to 'compiler/driver/compiler_driver.cc') diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index 9cc94e8c0d..4af492bf6e 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -513,10 +513,9 @@ void CompilerDriver::CompileAll(jobject class_loader, } } -static DexToDexCompilationLevel GetDexToDexCompilationlevel(mirror::ClassLoader* class_loader, - const DexFile& dex_file, - const DexFile::ClassDef& class_def) - SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { +static DexToDexCompilationLevel GetDexToDexCompilationlevel( + SirtRef& class_loader, const DexFile& dex_file, + const DexFile::ClassDef& class_def) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { const char* descriptor = dex_file.GetClassDescriptor(class_def); ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); mirror::Class* klass = class_linker->FindClass(descriptor, class_loader); @@ -531,7 +530,7 @@ static DexToDexCompilationLevel GetDexToDexCompilationlevel(mirror::ClassLoader* // function). Since image classes can be verified again while compiling an application, // we must prevent the DEX-to-DEX compiler from introducing them. // TODO: find a way to enable "quick" instructions for image classes and remove this check. - bool compiling_image_classes = (class_loader == NULL); + bool compiling_image_classes = class_loader.get() == nullptr; if (compiling_image_classes) { return kRequired; } else if (klass->IsVerified()) { @@ -579,7 +578,8 @@ void CompilerDriver::CompileOne(const mirror::ArtMethod* method, base::TimingLog { ScopedObjectAccess soa(Thread::Current()); const DexFile::ClassDef& class_def = dex_file->GetClassDef(class_def_idx); - mirror::ClassLoader* class_loader = soa.Decode(jclass_loader); + SirtRef class_loader(soa.Self(), + soa.Decode(jclass_loader)); dex_to_dex_compilation_level = GetDexToDexCompilationlevel(class_loader, *dex_file, class_def); } CompileMethod(code_item, method->GetAccessFlags(), method->GetInvokeType(), @@ -721,8 +721,8 @@ void CompilerDriver::LoadImageClasses(base::TimingLogger& timings) for (const std::pair& exception_type : unresolved_exception_types) { uint16_t exception_type_idx = exception_type.first; const DexFile* dex_file = exception_type.second; - mirror::DexCache* dex_cache = class_linker->FindDexCache(*dex_file); - mirror:: ClassLoader* class_loader = NULL; + SirtRef dex_cache(self, class_linker->FindDexCache(*dex_file)); + SirtRef class_loader(self, nullptr); SirtRef klass(self, class_linker->ResolveType(*dex_file, exception_type_idx, dex_cache, class_loader)); if (klass.get() == NULL) { @@ -782,15 +782,14 @@ void CompilerDriver::UpdateImageClasses(base::TimingLogger& timings) { const char* old_cause = self->StartAssertNoThreadSuspension("ImageWriter"); gc::Heap* heap = Runtime::Current()->GetHeap(); // TODO: Image spaces only? + ScopedObjectAccess soa(Thread::Current()); WriterMutexLock mu(self, *Locks::heap_bitmap_lock_); - heap->FlushAllocStack(); - heap->GetLiveBitmap()->Walk(FindClinitImageClassesCallback, this); + heap->VisitObjects(FindClinitImageClassesCallback, this); self->EndAssertNoThreadSuspension(old_cause); } } -bool CompilerDriver::CanAssumeTypeIsPresentInDexCache(const DexFile& dex_file, - uint32_t type_idx) { +bool CompilerDriver::CanAssumeTypeIsPresentInDexCache(const DexFile& dex_file, uint32_t type_idx) { if (IsImage() && IsImageClass(dex_file.StringDataByIdx(dex_file.GetTypeId(type_idx).descriptor_idx_))) { if (kIsDebugBuild) { @@ -815,7 +814,7 @@ bool CompilerDriver::CanAssumeStringIsPresentInDexCache(const DexFile& dex_file, if (IsImage()) { // We resolve all const-string strings when building for the image. ScopedObjectAccess soa(Thread::Current()); - mirror::DexCache* dex_cache = Runtime::Current()->GetClassLinker()->FindDexCache(dex_file); + SirtRef dex_cache(soa.Self(), Runtime::Current()->GetClassLinker()->FindDexCache(dex_file)); Runtime::Current()->GetClassLinker()->ResolveString(dex_file, string_idx, dex_cache); result = true; } @@ -903,26 +902,27 @@ bool CompilerDriver::CanAccessInstantiableTypeWithoutChecks(uint32_t referrer_id } static mirror::Class* ComputeCompilingMethodsClass(ScopedObjectAccess& soa, - mirror::DexCache* dex_cache, + SirtRef& dex_cache, const DexCompilationUnit* mUnit) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { // The passed dex_cache is a hint, sanity check before asking the class linker that will take a // lock. if (dex_cache->GetDexFile() != mUnit->GetDexFile()) { - dex_cache = mUnit->GetClassLinker()->FindDexCache(*mUnit->GetDexFile()); + dex_cache.reset(mUnit->GetClassLinker()->FindDexCache(*mUnit->GetDexFile())); } - mirror::ClassLoader* class_loader = soa.Decode(mUnit->GetClassLoader()); - const DexFile::MethodId& referrer_method_id = mUnit->GetDexFile()->GetMethodId(mUnit->GetDexMethodIndex()); + SirtRef + class_loader(soa.Self(), soa.Decode(mUnit->GetClassLoader())); + const DexFile::MethodId& referrer_method_id = + mUnit->GetDexFile()->GetMethodId(mUnit->GetDexMethodIndex()); return mUnit->GetClassLinker()->ResolveType(*mUnit->GetDexFile(), referrer_method_id.class_idx_, dex_cache, class_loader); } -static mirror::ArtField* ComputeFieldReferencedFromCompilingMethod(ScopedObjectAccess& soa, - const DexCompilationUnit* mUnit, - uint32_t field_idx) +static mirror::ArtField* ComputeFieldReferencedFromCompilingMethod( + ScopedObjectAccess& soa, const DexCompilationUnit* mUnit, uint32_t field_idx) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - mirror::DexCache* dex_cache = mUnit->GetClassLinker()->FindDexCache(*mUnit->GetDexFile()); - mirror::ClassLoader* class_loader = soa.Decode(mUnit->GetClassLoader()); + SirtRef dex_cache(soa.Self(), mUnit->GetClassLinker()->FindDexCache(*mUnit->GetDexFile())); + SirtRef class_loader(soa.Self(), soa.Decode(mUnit->GetClassLoader())); return mUnit->GetClassLinker()->ResolveField(*mUnit->GetDexFile(), field_idx, dex_cache, class_loader, false); } @@ -932,8 +932,8 @@ static mirror::ArtMethod* ComputeMethodReferencedFromCompilingMethod(ScopedObjec uint32_t method_idx, InvokeType type) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - mirror::DexCache* dex_cache = mUnit->GetClassLinker()->FindDexCache(*mUnit->GetDexFile()); - mirror::ClassLoader* class_loader = soa.Decode(mUnit->GetClassLoader()); + SirtRef dex_cache(soa.Self(), mUnit->GetClassLinker()->FindDexCache(*mUnit->GetDexFile())); + SirtRef class_loader(soa.Self(), soa.Decode(mUnit->GetClassLoader())); return mUnit->GetClassLinker()->ResolveMethod(*mUnit->GetDexFile(), method_idx, dex_cache, class_loader, NULL, type); } @@ -947,9 +947,10 @@ bool CompilerDriver::ComputeInstanceFieldInfo(uint32_t field_idx, const DexCompi // Try to resolve field and ignore if an Incompatible Class Change Error (ie is static). mirror::ArtField* resolved_field = ComputeFieldReferencedFromCompilingMethod(soa, mUnit, field_idx); if (resolved_field != NULL && !resolved_field->IsStatic()) { + SirtRef dex_cache(soa.Self(), + resolved_field->GetDeclaringClass()->GetDexCache()); mirror::Class* referrer_class = - ComputeCompilingMethodsClass(soa, resolved_field->GetDeclaringClass()->GetDexCache(), - mUnit); + ComputeCompilingMethodsClass(soa, dex_cache, mUnit); if (referrer_class != NULL) { mirror::Class* fields_class = resolved_field->GetDeclaringClass(); bool access_ok = referrer_class->CanAccess(fields_class) && @@ -997,9 +998,9 @@ bool CompilerDriver::ComputeStaticFieldInfo(uint32_t field_idx, const DexCompila // Try to resolve field and ignore if an Incompatible Class Change Error (ie isn't static). mirror::ArtField* resolved_field = ComputeFieldReferencedFromCompilingMethod(soa, mUnit, field_idx); if (resolved_field != NULL && resolved_field->IsStatic()) { + SirtRef dex_cache(soa.Self(), resolved_field->GetDeclaringClass()->GetDexCache()); mirror::Class* referrer_class = - ComputeCompilingMethodsClass(soa, resolved_field->GetDeclaringClass()->GetDexCache(), - mUnit); + ComputeCompilingMethodsClass(soa, dex_cache, mUnit); if (referrer_class != NULL) { mirror::Class* fields_class = resolved_field->GetDeclaringClass(); if (fields_class == referrer_class) { @@ -1085,7 +1086,7 @@ void CompilerDriver::GetCodeAndMethodForDirectCall(InvokeType* type, InvokeType *direct_code = 0; *direct_method = 0; bool use_dex_cache = false; - bool compiling_boot = Runtime::Current()->GetHeap()->GetContinuousSpaces().size() == 1; + const bool compiling_boot = Runtime::Current()->GetHeap()->IsCompilingBoot(); if (compiler_backend_ == kPortable) { if (sharp_type != kStatic && sharp_type != kDirect) { return; @@ -1198,9 +1199,9 @@ bool CompilerDriver::ComputeInvokeInfo(const DexCompilationUnit* mUnit, const ui } // Don't try to fast-path if we don't understand the caller's class or this appears to be an // Incompatible Class Change Error. + SirtRef dex_cache(soa.Self(), resolved_method->GetDeclaringClass()->GetDexCache()); mirror::Class* referrer_class = - ComputeCompilingMethodsClass(soa, resolved_method->GetDeclaringClass()->GetDexCache(), - mUnit); + ComputeCompilingMethodsClass(soa, dex_cache, mUnit); bool icce = resolved_method->CheckIncompatibleClassChange(*invoke_type); if (referrer_class != NULL && !icce) { mirror::Class* methods_class = resolved_method->GetDeclaringClass(); @@ -1254,10 +1255,8 @@ bool CompilerDriver::ComputeInvokeInfo(const DexCompilationUnit* mUnit, const ui const MethodReference* devirt_map_target = verifier::MethodVerifier::GetDevirtMap(caller_method, dex_pc); if (devirt_map_target != NULL) { - mirror::DexCache* target_dex_cache = - mUnit->GetClassLinker()->FindDexCache(*devirt_map_target->dex_file); - mirror::ClassLoader* class_loader = - soa.Decode(mUnit->GetClassLoader()); + SirtRef target_dex_cache(soa.Self(), mUnit->GetClassLinker()->FindDexCache(*devirt_map_target->dex_file)); + SirtRef class_loader(soa.Self(), soa.Decode(mUnit->GetClassLoader())); mirror::ArtMethod* called_method = mUnit->GetClassLinker()->ResolveMethod(*devirt_map_target->dex_file, devirt_map_target->dex_method_index, @@ -1509,13 +1508,11 @@ static void ResolveClassFieldsAndMethods(const ParallelCompilationManager* manag const DexFile::ClassDef& class_def = dex_file.GetClassDef(class_def_index); if (!SkipClass(class_linker, jclass_loader, dex_file, class_def)) { ScopedObjectAccess soa(self); - mirror::ClassLoader* class_loader = soa.Decode(jclass_loader); - mirror::DexCache* dex_cache = class_linker->FindDexCache(dex_file); - + SirtRef class_loader(soa.Self(), soa.Decode(jclass_loader)); + SirtRef dex_cache(soa.Self(), class_linker->FindDexCache(dex_file)); // Resolve the class. mirror::Class* klass = class_linker->ResolveType(dex_file, class_def.class_idx_, dex_cache, class_loader); - bool resolve_fields_and_methods; if (klass == NULL) { // Class couldn't be resolved, for example, super-class is in a different dex file. Don't @@ -1598,8 +1595,8 @@ static void ResolveType(const ParallelCompilationManager* manager, size_t type_i ScopedObjectAccess soa(Thread::Current()); ClassLinker* class_linker = manager->GetClassLinker(); const DexFile& dex_file = *manager->GetDexFile(); - mirror::DexCache* dex_cache = class_linker->FindDexCache(dex_file); - mirror::ClassLoader* class_loader = soa.Decode(manager->GetClassLoader()); + SirtRef dex_cache(soa.Self(), class_linker->FindDexCache(dex_file)); + SirtRef class_loader(soa.Self(), soa.Decode(manager->GetClassLoader())); mirror::Class* klass = class_linker->ResolveType(dex_file, type_idx, dex_cache, class_loader); if (klass == NULL) { @@ -1652,8 +1649,9 @@ static void VerifyClass(const ParallelCompilationManager* manager, size_t class_ const char* descriptor = dex_file.GetClassDescriptor(class_def); ClassLinker* class_linker = manager->GetClassLinker(); jobject jclass_loader = manager->GetClassLoader(); - mirror::Class* klass = class_linker->FindClass(descriptor, - soa.Decode(jclass_loader)); + SirtRef class_loader( + soa.Self(), soa.Decode(jclass_loader)); + mirror::Class* klass = class_linker->FindClass(descriptor, class_loader); if (klass == NULL) { CHECK(soa.Self()->IsExceptionPending()); soa.Self()->ClearException(); @@ -1663,11 +1661,10 @@ static void VerifyClass(const ParallelCompilationManager* manager, size_t class_ * This is to ensure the class is structurally sound for compilation. An unsound class * will be rejected by the verifier and later skipped during compilation in the compiler. */ - mirror::DexCache* dex_cache = class_linker->FindDexCache(dex_file); + SirtRef dex_cache(soa.Self(), class_linker->FindDexCache(dex_file)); std::string error_msg; - if (verifier::MethodVerifier::VerifyClass(&dex_file, dex_cache, - soa.Decode(jclass_loader), - &class_def, true, &error_msg) == + if (verifier::MethodVerifier::VerifyClass(&dex_file, dex_cache, class_loader, &class_def, true, + &error_msg) == verifier::MethodVerifier::kHardFailure) { LOG(ERROR) << "Verification failed on class " << PrettyDescriptor(descriptor) << " because: " << error_msg; @@ -2124,7 +2121,8 @@ static void InitializeClass(const ParallelCompilationManager* manager, size_t cl const char* descriptor = dex_file.StringDataByIdx(class_type_id.descriptor_idx_); ScopedObjectAccess soa(Thread::Current()); - mirror::ClassLoader* class_loader = soa.Decode(jclass_loader); + SirtRef class_loader(soa.Self(), + soa.Decode(jclass_loader)); mirror::Class* klass = manager->GetClassLinker()->FindClass(descriptor, class_loader); if (klass != NULL && !SkipClass(jclass_loader, dex_file, klass)) { @@ -2253,7 +2251,8 @@ void CompilerDriver::CompileClass(const ParallelCompilationManager* manager, siz DexToDexCompilationLevel dex_to_dex_compilation_level = kDontDexToDexCompile; { ScopedObjectAccess soa(Thread::Current()); - mirror::ClassLoader* class_loader = soa.Decode(jclass_loader); + SirtRef class_loader(soa.Self(), + soa.Decode(jclass_loader)); dex_to_dex_compilation_level = GetDexToDexCompilationlevel(class_loader, dex_file, class_def); } ClassDataItemIterator it(dex_file, class_data); -- cgit v1.2.3-59-g8ed1b From bcd5e9daecad39f0dab3246808b4835caec29ea6 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Wed, 13 Nov 2013 14:33:28 -0800 Subject: Manually manage thread pool stacks. We now allocate the thread pool worker stack using a MemMap. This enables us to name the maps so that we get more descriptive output for debugging leaks. Appears to fix the mips build 5/5 successful clean-oat and builds. This is probably since glibc caches up to 40 MB of thread stacks before releasing them. Change-Id: I1df2de50cb95838aa0d272a09807021404ba410c --- compiler/driver/compiler_driver.cc | 4 ++-- runtime/barrier_test.cc | 4 ++-- runtime/gc/heap.cc | 2 +- runtime/thread_pool.cc | 20 ++++++++++++-------- runtime/thread_pool.h | 11 +++++++---- runtime/thread_pool_test.cc | 6 +++--- 6 files changed, 27 insertions(+), 20 deletions(-) (limited to 'compiler/driver/compiler_driver.cc') diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index 4af492bf6e..d74383e33b 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -505,7 +505,7 @@ void CompilerDriver::CompileAll(jobject class_loader, const std::vector& dex_files, base::TimingLogger& timings) { DCHECK(!Runtime::Current()->IsStarted()); - UniquePtr thread_pool(new ThreadPool(thread_count_ - 1)); + UniquePtr thread_pool(new ThreadPool("Compiler driver thread pool", thread_count_ - 1)); PreCompile(class_loader, dex_files, *thread_pool.get(), timings); Compile(class_loader, dex_files, *thread_pool.get(), timings); if (dump_stats_) { @@ -568,7 +568,7 @@ void CompilerDriver::CompileOne(const mirror::ArtMethod* method, base::TimingLog std::vector dex_files; dex_files.push_back(dex_file); - UniquePtr thread_pool(new ThreadPool(0U)); + UniquePtr thread_pool(new ThreadPool("Compiler driver thread pool", 0U)); PreCompile(jclass_loader, dex_files, *thread_pool.get(), timings); uint32_t method_idx = method->GetDexMethodIndex(); diff --git a/runtime/barrier_test.cc b/runtime/barrier_test.cc index 298ae569fb..91fc143dbe 100644 --- a/runtime/barrier_test.cc +++ b/runtime/barrier_test.cc @@ -66,7 +66,7 @@ int32_t BarrierTest::num_threads = 4; // Check that barrier wait and barrier increment work. TEST_F(BarrierTest, CheckWait) { Thread* self = Thread::Current(); - ThreadPool thread_pool(num_threads); + ThreadPool thread_pool("Barrier test thread pool", num_threads); Barrier barrier(0); AtomicInteger count1(0); AtomicInteger count2(0); @@ -121,7 +121,7 @@ class CheckPassTask : public Task { // Check that barrier pass through works. TEST_F(BarrierTest, CheckPass) { Thread* self = Thread::Current(); - ThreadPool thread_pool(num_threads); + ThreadPool thread_pool("Barrier test thread pool", num_threads); Barrier barrier(0); AtomicInteger count(0); const int32_t num_tasks = num_threads * 4; diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc index 69ca6202f9..70df3d3133 100644 --- a/runtime/gc/heap.cc +++ b/runtime/gc/heap.cc @@ -309,7 +309,7 @@ void Heap::DecrementDisableGC(Thread* self) { void Heap::CreateThreadPool() { const size_t num_threads = std::max(parallel_gc_threads_, conc_gc_threads_); if (num_threads != 0) { - thread_pool_.reset(new ThreadPool(num_threads)); + thread_pool_.reset(new ThreadPool("Heap thread pool", num_threads)); } } diff --git a/runtime/thread_pool.cc b/runtime/thread_pool.cc index bb6c47538e..aca0561a77 100644 --- a/runtime/thread_pool.cc +++ b/runtime/thread_pool.cc @@ -28,12 +28,15 @@ static constexpr bool kMeasureWaitTime = false; ThreadPoolWorker::ThreadPoolWorker(ThreadPool* thread_pool, const std::string& name, size_t stack_size) : thread_pool_(thread_pool), - name_(name), - stack_size_(stack_size) { + name_(name) { + std::string error_msg; + stack_.reset(MemMap::MapAnonymous(name.c_str(), nullptr, stack_size, PROT_READ | PROT_WRITE, + &error_msg)); + CHECK(stack_.get() != nullptr) << error_msg; const char* reason = "new thread pool worker thread"; pthread_attr_t attr; CHECK_PTHREAD_CALL(pthread_attr_init, (&attr), reason); - CHECK_PTHREAD_CALL(pthread_attr_setstacksize, (&attr, stack_size), reason); + CHECK_PTHREAD_CALL(pthread_attr_setstack, (&attr, stack_->Begin(), stack_->Size()), reason); CHECK_PTHREAD_CALL(pthread_create, (&pthread_, &attr, &Callback, this), reason); CHECK_PTHREAD_CALL(pthread_attr_destroy, (&attr), reason); } @@ -71,8 +74,9 @@ void ThreadPool::AddTask(Thread* self, Task* task) { } } -ThreadPool::ThreadPool(size_t num_threads) - : task_queue_lock_("task queue lock"), +ThreadPool::ThreadPool(const char* name, size_t num_threads) + : name_(name), + task_queue_lock_("task queue lock"), task_queue_condition_("task queue condition", task_queue_lock_), completion_condition_("task completion condition", task_queue_lock_), started_(false), @@ -85,7 +89,7 @@ ThreadPool::ThreadPool(size_t num_threads) max_active_workers_(num_threads) { Thread* self = Thread::Current(); while (GetThreadCount() < num_threads) { - const std::string name = StringPrintf("Thread pool worker %zu", GetThreadCount()); + const std::string name = StringPrintf("%s worker thread %zu", name_.c_str(), GetThreadCount()); threads_.push_back(new ThreadPoolWorker(this, name, ThreadPoolWorker::kDefaultStackSize)); } // Wait for all of the threads to attach. @@ -270,8 +274,8 @@ void WorkStealingWorker::Run() { WorkStealingWorker::~WorkStealingWorker() {} -WorkStealingThreadPool::WorkStealingThreadPool(size_t num_threads) - : ThreadPool(0), +WorkStealingThreadPool::WorkStealingThreadPool(const char* name, size_t num_threads) + : ThreadPool(name, 0), work_steal_lock_("work stealing lock"), steal_index_(0) { while (GetThreadCount() < num_threads) { diff --git a/runtime/thread_pool.h b/runtime/thread_pool.h index b9a97a1427..e8f9afe62d 100644 --- a/runtime/thread_pool.h +++ b/runtime/thread_pool.h @@ -24,6 +24,7 @@ #include "base/mutex.h" #include "closure.h" #include "locks.h" +#include "mem_map.h" namespace art { @@ -40,7 +41,8 @@ class ThreadPoolWorker { static const size_t kDefaultStackSize = 1 * MB; size_t GetStackSize() const { - return stack_size_; + DCHECK(stack_.get() != nullptr); + return stack_->Size(); } virtual ~ThreadPoolWorker(); @@ -52,7 +54,7 @@ class ThreadPoolWorker { ThreadPool* const thread_pool_; const std::string name_; - const size_t stack_size_; + UniquePtr stack_; pthread_t pthread_; private: @@ -77,7 +79,7 @@ class ThreadPool { // after running it, it is the caller's responsibility. void AddTask(Thread* self, Task* task); - explicit ThreadPool(size_t num_threads); + explicit ThreadPool(const char* name, size_t num_threads); virtual ~ThreadPool(); // Wait for all tasks currently on queue to get completed. @@ -107,6 +109,7 @@ class ThreadPool { return shutting_down_; } + const std::string name_; Mutex task_queue_lock_; ConditionVariable task_queue_condition_ GUARDED_BY(task_queue_lock_); ConditionVariable completion_condition_ GUARDED_BY(task_queue_lock_); @@ -167,7 +170,7 @@ class WorkStealingWorker : public ThreadPoolWorker { class WorkStealingThreadPool : public ThreadPool { public: - explicit WorkStealingThreadPool(size_t num_threads); + explicit WorkStealingThreadPool(const char* name, size_t num_threads); virtual ~WorkStealingThreadPool(); private: diff --git a/runtime/thread_pool_test.cc b/runtime/thread_pool_test.cc index 9b789d2f4c..1b2236152f 100644 --- a/runtime/thread_pool_test.cc +++ b/runtime/thread_pool_test.cc @@ -59,7 +59,7 @@ int32_t ThreadPoolTest::num_threads = 4; // Check that the thread pool actually runs tasks that you assign it. TEST_F(ThreadPoolTest, CheckRun) { Thread* self = Thread::Current(); - ThreadPool thread_pool(num_threads); + ThreadPool thread_pool("Thread pool test thread pool", num_threads); AtomicInteger count(0); static const int32_t num_tasks = num_threads * 4; for (int32_t i = 0; i < num_tasks; ++i) { @@ -74,7 +74,7 @@ TEST_F(ThreadPoolTest, CheckRun) { TEST_F(ThreadPoolTest, StopStart) { Thread* self = Thread::Current(); - ThreadPool thread_pool(num_threads); + ThreadPool thread_pool("Thread pool test thread pool", num_threads); AtomicInteger count(0); static const int32_t num_tasks = num_threads * 4; for (int32_t i = 0; i < num_tasks; ++i) { @@ -129,7 +129,7 @@ class TreeTask : public Task { // Test that adding new tasks from within a task works. TEST_F(ThreadPoolTest, RecursiveTest) { Thread* self = Thread::Current(); - ThreadPool thread_pool(num_threads); + ThreadPool thread_pool("Thread pool test thread pool", num_threads); AtomicInteger count(0); static const int depth = 8; thread_pool.AddTask(self, new TreeTask(&thread_pool, &count, depth)); -- cgit v1.2.3-59-g8ed1b From 5fe9af720048673e62ee29597a30bb9e54c903c5 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 14 Nov 2013 00:17:20 -0800 Subject: Fix memory leaks relating to timing logger. Bug: 11670287. We use pointers to uninitialized values for control-flow in the timing logger code, add TODO comments to clean this up later. Remove base namespace and other bits of tidying. Change-Id: I1e6600a1e92f974c8f58f3a405a4e4abb4d9f80f --- compiler/dex/compiler_ir.h | 2 +- compiler/dex/frontend.cc | 2 +- compiler/driver/compiler_driver.cc | 26 +++++++------- compiler/driver/compiler_driver.h | 26 +++++++------- compiler/driver/compiler_driver_test.cc | 3 +- compiler/image_test.cc | 3 +- compiler/oat_test.cc | 4 +-- dex2oat/dex2oat.cc | 8 ++--- runtime/base/timing_logger.cc | 58 +++++++++++++++----------------- runtime/base/timing_logger.h | 18 ++++------ runtime/base/timing_logger_test.cc | 30 ++++++++--------- runtime/common_test.h | 3 +- runtime/gc/collector/garbage_collector.h | 4 +-- runtime/gc/collector/mark_sweep.cc | 30 ++++++++--------- runtime/gc/collector/semi_space.cc | 22 ++++++------ runtime/gc/heap.cc | 8 ++--- runtime/gc/heap.h | 2 +- 17 files changed, 121 insertions(+), 128 deletions(-) (limited to 'compiler/driver/compiler_driver.cc') diff --git a/compiler/dex/compiler_ir.h b/compiler/dex/compiler_ir.h index 546ce4aee7..3798b459d1 100644 --- a/compiler/dex/compiler_ir.h +++ b/compiler/dex/compiler_ir.h @@ -94,7 +94,7 @@ struct CompilationUnit { UniquePtr mir_graph; // MIR container. UniquePtr cg; // Target-specific codegen. - base::TimingLogger timings; + TimingLogger timings; }; } // namespace art diff --git a/compiler/dex/frontend.cc b/compiler/dex/frontend.cc index b8cd67e3e7..c643bf85d9 100644 --- a/compiler/dex/frontend.cc +++ b/compiler/dex/frontend.cc @@ -157,7 +157,7 @@ void CompilationUnit::EndTiming() { if (enable_debug & (1 << kDebugTimings)) { timings.EndSplit(); LOG(INFO) << "TIMINGS " << PrettyMethod(method_idx, *dex_file); - LOG(INFO) << Dumpable(timings); + LOG(INFO) << Dumpable(timings); } } diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index d74383e33b..b9df1d6f48 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -503,7 +503,7 @@ const std::vector* CompilerDriver::CreateQuickToInterpreterBridge() con void CompilerDriver::CompileAll(jobject class_loader, const std::vector& dex_files, - base::TimingLogger& timings) { + TimingLogger& timings) { DCHECK(!Runtime::Current()->IsStarted()); UniquePtr thread_pool(new ThreadPool("Compiler driver thread pool", thread_count_ - 1)); PreCompile(class_loader, dex_files, *thread_pool.get(), timings); @@ -546,7 +546,7 @@ static DexToDexCompilationLevel GetDexToDexCompilationlevel( } } -void CompilerDriver::CompileOne(const mirror::ArtMethod* method, base::TimingLogger& timings) { +void CompilerDriver::CompileOne(const mirror::ArtMethod* method, TimingLogger& timings) { DCHECK(!Runtime::Current()->IsStarted()); Thread* self = Thread::Current(); jobject jclass_loader; @@ -591,7 +591,7 @@ void CompilerDriver::CompileOne(const mirror::ArtMethod* method, base::TimingLog } void CompilerDriver::Resolve(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { for (size_t i = 0; i != dex_files.size(); ++i) { const DexFile* dex_file = dex_files[i]; CHECK(dex_file != NULL); @@ -600,7 +600,7 @@ void CompilerDriver::Resolve(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { LoadImageClasses(timings); Resolve(class_loader, dex_files, thread_pool, timings); @@ -685,7 +685,7 @@ static bool RecordImageClassesVisitor(mirror::Class* klass, void* arg) } // Make a list of descriptors for classes to include in the image -void CompilerDriver::LoadImageClasses(base::TimingLogger& timings) +void CompilerDriver::LoadImageClasses(TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_) { if (!IsImage()) { return; @@ -773,7 +773,7 @@ void CompilerDriver::FindClinitImageClassesCallback(mirror::Object* object, void MaybeAddToImageClasses(object->GetClass(), compiler_driver->image_classes_.get()); } -void CompilerDriver::UpdateImageClasses(base::TimingLogger& timings) { +void CompilerDriver::UpdateImageClasses(TimingLogger& timings) { if (IsImage()) { timings.NewSplit("UpdateImageClasses"); @@ -1613,7 +1613,7 @@ static void ResolveType(const ParallelCompilationManager* manager, size_t type_i } void CompilerDriver::ResolveDexFile(jobject class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); // TODO: we could resolve strings here, although the string table is largely filled with class @@ -1632,7 +1632,7 @@ void CompilerDriver::ResolveDexFile(jobject class_loader, const DexFile& dex_fil } void CompilerDriver::Verify(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { for (size_t i = 0; i != dex_files.size(); ++i) { const DexFile* dex_file = dex_files[i]; CHECK(dex_file != NULL); @@ -1686,7 +1686,7 @@ static void VerifyClass(const ParallelCompilationManager* manager, size_t class_ } void CompilerDriver::VerifyDexFile(jobject class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { timings.NewSplit("Verify Dex File"); ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); ParallelCompilationManager context(class_linker, class_loader, this, &dex_file, thread_pool); @@ -2192,7 +2192,7 @@ static void InitializeClass(const ParallelCompilationManager* manager, size_t cl } void CompilerDriver::InitializeClasses(jobject jni_class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { timings.NewSplit("InitializeNoClinit"); #ifndef NDEBUG // Sanity check blacklist descriptors. @@ -2210,7 +2210,7 @@ void CompilerDriver::InitializeClasses(jobject jni_class_loader, const DexFile& void CompilerDriver::InitializeClasses(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { for (size_t i = 0; i != dex_files.size(); ++i) { const DexFile* dex_file = dex_files[i]; CHECK(dex_file != NULL); @@ -2219,7 +2219,7 @@ void CompilerDriver::InitializeClasses(jobject class_loader, } void CompilerDriver::Compile(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { for (size_t i = 0; i != dex_files.size(); ++i) { const DexFile* dex_file = dex_files[i]; CHECK(dex_file != NULL); @@ -2300,7 +2300,7 @@ void CompilerDriver::CompileClass(const ParallelCompilationManager* manager, siz } void CompilerDriver::CompileDexFile(jobject class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) { + ThreadPool& thread_pool, TimingLogger& timings) { timings.NewSplit("Compile Dex File"); ParallelCompilationManager context(Runtime::Current()->GetClassLinker(), class_loader, this, &dex_file, thread_pool); diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h index 9bfea6ff0a..7e8184975c 100644 --- a/compiler/driver/compiler_driver.h +++ b/compiler/driver/compiler_driver.h @@ -98,11 +98,11 @@ class CompilerDriver { ~CompilerDriver(); void CompileAll(jobject class_loader, const std::vector& dex_files, - base::TimingLogger& timings) + TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); // Compile a single Method - void CompileOne(const mirror::ArtMethod* method, base::TimingLogger& timings) + void CompileOne(const mirror::ArtMethod* method, TimingLogger& timings) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); const InstructionSet& GetInstructionSet() const { @@ -340,43 +340,43 @@ class CompilerDriver { SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void PreCompile(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) + ThreadPool& thread_pool, TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); - void LoadImageClasses(base::TimingLogger& timings); + void LoadImageClasses(TimingLogger& timings); // Attempt to resolve all type, methods, fields, and strings // referenced from code in the dex file following PathClassLoader // ordering semantics. void Resolve(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) + ThreadPool& thread_pool, TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); void ResolveDexFile(jobject class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) + ThreadPool& thread_pool, TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); void Verify(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings); + ThreadPool& thread_pool, TimingLogger& timings); void VerifyDexFile(jobject class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) + ThreadPool& thread_pool, TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); void InitializeClasses(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings) + ThreadPool& thread_pool, TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); void InitializeClasses(jobject class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) + ThreadPool& thread_pool, TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_, compiled_classes_lock_); - void UpdateImageClasses(base::TimingLogger& timings) + void UpdateImageClasses(TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); static void FindClinitImageClassesCallback(mirror::Object* object, void* arg) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void Compile(jobject class_loader, const std::vector& dex_files, - ThreadPool& thread_pool, base::TimingLogger& timings); + ThreadPool& thread_pool, TimingLogger& timings); void CompileDexFile(jobject class_loader, const DexFile& dex_file, - ThreadPool& thread_pool, base::TimingLogger& timings) + ThreadPool& thread_pool, TimingLogger& timings) LOCKS_EXCLUDED(Locks::mutator_lock_); void CompileMethod(const DexFile::CodeItem* code_item, uint32_t access_flags, InvokeType invoke_type, uint16_t class_def_idx, uint32_t method_idx, diff --git a/compiler/driver/compiler_driver_test.cc b/compiler/driver/compiler_driver_test.cc index bfc93b3c8f..a5eb94f0e9 100644 --- a/compiler/driver/compiler_driver_test.cc +++ b/compiler/driver/compiler_driver_test.cc @@ -36,12 +36,13 @@ namespace art { class CompilerDriverTest : public CommonTest { protected: void CompileAll(jobject class_loader) LOCKS_EXCLUDED(Locks::mutator_lock_) { - base::TimingLogger timings("CompilerDriverTest::CompileAll", false, false); + TimingLogger timings("CompilerDriverTest::CompileAll", false, false); timings.StartSplit("CompileAll"); compiler_driver_->CompileAll(class_loader, Runtime::Current()->GetCompileTimeClassPath(class_loader), timings); MakeAllExecutable(class_loader); + timings.EndSplit(); } void EnsureCompiled(jobject class_loader, const char* class_name, const char* method, diff --git a/compiler/image_test.cc b/compiler/image_test.cc index 9d9c06401e..e22e7028f6 100644 --- a/compiler/image_test.cc +++ b/compiler/image_test.cc @@ -46,7 +46,7 @@ TEST_F(ImageTest, WriteRead) { { jobject class_loader = NULL; ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); - base::TimingLogger timings("ImageTest::WriteRead", false, false); + TimingLogger timings("ImageTest::WriteRead", false, false); timings.StartSplit("CompileAll"); #if defined(ART_USE_PORTABLE_COMPILER) // TODO: we disable this for portable so the test executes in a reasonable amount of time. @@ -67,6 +67,7 @@ TEST_F(ImageTest, WriteRead) { oat_writer, tmp_elf.GetFile()); ASSERT_TRUE(success); + timings.EndSplit(); } } // Workound bug that mcld::Linker::emit closes tmp_elf by reopening as tmp_oat. diff --git a/compiler/oat_test.cc b/compiler/oat_test.cc index c423f34f7f..714ec4eee8 100644 --- a/compiler/oat_test.cc +++ b/compiler/oat_test.cc @@ -82,7 +82,7 @@ TEST_F(OatTest, WriteRead) { insn_features, false, NULL, 2, true)); jobject class_loader = NULL; if (kCompile) { - base::TimingLogger timings("OatTest::WriteRead", false, false); + TimingLogger timings("OatTest::WriteRead", false, false); compiler_driver_->CompileAll(class_loader, class_linker->GetBootClassPath(), timings); } @@ -101,7 +101,7 @@ TEST_F(OatTest, WriteRead) { ASSERT_TRUE(success); if (kCompile) { // OatWriter strips the code, regenerate to compare - base::TimingLogger timings("CommonTest::WriteRead", false, false); + TimingLogger timings("CommonTest::WriteRead", false, false); compiler_driver_->CompileAll(class_loader, class_linker->GetBootClassPath(), timings); } std::string error_msg; diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 3781921927..fada66a952 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -242,7 +242,7 @@ class Dex2Oat { bool image, UniquePtr& image_classes, bool dump_stats, - base::TimingLogger& timings) { + TimingLogger& timings) { // SirtRef and ClassLoader creation needs to come after Runtime::Create jobject class_loader = NULL; Thread* self = Thread::Current(); @@ -600,7 +600,7 @@ static InstructionSetFeatures ParseFeatureList(std::string str) { } static int dex2oat(int argc, char** argv) { - base::TimingLogger timings("compiler", false, false); + TimingLogger timings("compiler", false, false); InitLogging(argv); @@ -1091,7 +1091,7 @@ static int dex2oat(int argc, char** argv) { if (is_host) { if (dump_timing || (dump_slow_timing && timings.GetTotalNs() > MsToNs(1000))) { - LOG(INFO) << Dumpable(timings); + LOG(INFO) << Dumpable(timings); } return EXIT_SUCCESS; } @@ -1133,7 +1133,7 @@ static int dex2oat(int argc, char** argv) { timings.EndSplit(); if (dump_timing || (dump_slow_timing && timings.GetTotalNs() > MsToNs(1000))) { - LOG(INFO) << Dumpable(timings); + LOG(INFO) << Dumpable(timings); } // Everything was successfully written, do an explicit exit here to avoid running Runtime diff --git a/runtime/base/timing_logger.cc b/runtime/base/timing_logger.cc index 45a546f37e..dae8201117 100644 --- a/runtime/base/timing_logger.cc +++ b/runtime/base/timing_logger.cc @@ -74,12 +74,11 @@ uint64_t CumulativeLogger::GetTotalTime() const { return total; } -void CumulativeLogger::AddLogger(const base::TimingLogger &logger) { +void CumulativeLogger::AddLogger(const TimingLogger &logger) { MutexLock mu(Thread::Current(), lock_); - const base::TimingLogger::SplitTimings& splits = logger.GetSplits(); - for (base::TimingLogger::SplitTimingsIterator it = splits.begin(), end = splits.end(); - it != end; ++it) { - base::TimingLogger::SplitTiming split = *it; + const TimingLogger::SplitTimings& splits = logger.GetSplits(); + for (auto it = splits.begin(), end = splits.end(); it != end; ++it) { + TimingLogger::SplitTiming split = *it; uint64_t split_time = split.first; const char* split_name = split.second; AddPair(split_name, split_time); @@ -101,7 +100,8 @@ void CumulativeLogger::AddPair(const std::string &label, uint64_t delta_time) { delta_time /= kAdjust; if (histograms_.find(label) == histograms_.end()) { - // TODO: Shoud this be a defined constant so we we know out of which orifice 16 and 100 were picked? + // TODO: Should this be a defined constant so we we know out of which orifice 16 and 100 were + // picked? const size_t max_buckets = Runtime::Current()->GetHeap()->IsLowMemoryMode() ? 16 : 100; // TODO: Should this be a defined constant so we know 50 of WTF? histograms_[label] = new Histogram(label.c_str(), 50, max_buckets); @@ -123,9 +123,6 @@ void CumulativeLogger::DumpHistogram(std::ostream &os) { os << "Done Dumping histograms \n"; } - -namespace base { - TimingLogger::TimingLogger(const char* name, bool precise, bool verbose) : name_(name), precise_(precise), verbose_(verbose), current_split_(NULL) { } @@ -136,33 +133,35 @@ void TimingLogger::Reset() { } void TimingLogger::StartSplit(const char* new_split_label) { - DCHECK(new_split_label != NULL) << "Starting split (" << new_split_label << ") with null label."; - TimingLogger::ScopedSplit* explicit_scoped_split = new TimingLogger::ScopedSplit(new_split_label, this); + DCHECK(new_split_label != nullptr) << "Starting split with null label."; + TimingLogger::ScopedSplit* explicit_scoped_split = + new TimingLogger::ScopedSplit(new_split_label, this); explicit_scoped_split->explicit_ = true; } void TimingLogger::EndSplit() { - CHECK(current_split_ != NULL) << "Ending a non-existent split."; - DCHECK(current_split_->label_ != NULL); - DCHECK(current_split_->explicit_ == true) << "Explicitly ending scoped split: " << current_split_->label_; - + CHECK(current_split_ != nullptr) << "Ending a non-existent split."; + DCHECK(current_split_->label_ != nullptr); + DCHECK(current_split_->explicit_ == true) + << "Explicitly ending scoped split: " << current_split_->label_; delete current_split_; + // TODO: current_split_ = nullptr; } // Ends the current split and starts the one given by the label. void TimingLogger::NewSplit(const char* new_split_label) { - CHECK(current_split_ != NULL) << "Inserting a new split (" << new_split_label - << ") into a non-existent split."; - DCHECK(new_split_label != NULL) << "New split (" << new_split_label << ") with null label."; - - current_split_->TailInsertSplit(new_split_label); + if (current_split_ == nullptr) { + StartSplit(new_split_label); + } else { + DCHECK(new_split_label != nullptr) << "New split (" << new_split_label << ") with null label."; + current_split_->TailInsertSplit(new_split_label); + } } uint64_t TimingLogger::GetTotalNs() const { uint64_t total_ns = 0; - for (base::TimingLogger::SplitTimingsIterator it = splits_.begin(), end = splits_.end(); - it != end; ++it) { - base::TimingLogger::SplitTiming split = *it; + for (auto it = splits_.begin(), end = splits_.end(); it != end; ++it) { + TimingLogger::SplitTiming split = *it; total_ns += split.first; } return total_ns; @@ -171,9 +170,8 @@ uint64_t TimingLogger::GetTotalNs() const { void TimingLogger::Dump(std::ostream &os) const { uint64_t longest_split = 0; uint64_t total_ns = 0; - for (base::TimingLogger::SplitTimingsIterator it = splits_.begin(), end = splits_.end(); - it != end; ++it) { - base::TimingLogger::SplitTiming split = *it; + for (auto it = splits_.begin(), end = splits_.end(); it != end; ++it) { + TimingLogger::SplitTiming split = *it; uint64_t split_time = split.first; longest_split = std::max(longest_split, split_time); total_ns += split_time; @@ -182,9 +180,8 @@ void TimingLogger::Dump(std::ostream &os) const { TimeUnit tu = GetAppropriateTimeUnit(longest_split); uint64_t divisor = GetNsToTimeUnitDivisor(tu); // Print formatted splits. - for (base::TimingLogger::SplitTimingsIterator it = splits_.begin(), end = splits_.end(); - it != end; ++it) { - base::TimingLogger::SplitTiming split = *it; + for (auto it = splits_.begin(), end = splits_.end(); it != end; ++it) { + const TimingLogger::SplitTiming& split = *it; uint64_t split_time = split.first; if (!precise_ && divisor >= 1000) { // Make the fractional part 0. @@ -231,7 +228,7 @@ TimingLogger::ScopedSplit::~ScopedSplit() { LOG(INFO) << "End: " << label_ << " " << PrettyDuration(split_time); } - // If one or more enclosed explcitly started splits are not terminated we can + // If one or more enclosed explicitly started splits are not terminated we can // either fail or "unwind" the stack of splits in the timing logger to 'this' // (by deleting the intervening scoped splits). This implements the latter. TimingLogger::ScopedSplit* current = timing_logger_->current_split_; @@ -293,5 +290,4 @@ void TimingLogger::ScopedSplit::Resume() { ATRACE_BEGIN(label_); } -} // namespace base } // namespace art diff --git a/runtime/base/timing_logger.h b/runtime/base/timing_logger.h index 501d2d7fd2..f1f78557aa 100644 --- a/runtime/base/timing_logger.h +++ b/runtime/base/timing_logger.h @@ -26,10 +26,7 @@ #include namespace art { - -namespace base { - class TimingLogger; -} // namespace base +class TimingLogger; class CumulativeLogger { public: @@ -44,7 +41,7 @@ class CumulativeLogger { // Allow the name to be modified, particularly when the cumulative logger is a field within a // parent class that is unable to determine the "name" of a sub-class. void SetName(const std::string& name); - void AddLogger(const base::TimingLogger& logger) LOCKS_EXCLUDED(lock_); + void AddLogger(const TimingLogger& logger) LOCKS_EXCLUDED(lock_); size_t GetIterations() const; private: @@ -65,19 +62,17 @@ class CumulativeLogger { DISALLOW_COPY_AND_ASSIGN(CumulativeLogger); }; -namespace base { - - // A timing logger that knows when a split starts for the purposes of logging tools, like systrace. class TimingLogger { public: // Splits are nanosecond times and split names. typedef std::pair SplitTiming; typedef std::vector SplitTimings; - typedef std::vector::const_iterator SplitTimingsIterator; explicit TimingLogger(const char* name, bool precise, bool verbose); - + ~TimingLogger() { + // TODO: DCHECK(current_split_ == nullptr) << "Forgot to end split: " << current_split_->label_; + } // Clears current splits and labels. void Reset(); @@ -143,7 +138,7 @@ class TimingLogger { friend class ScopedSplit; protected: // The name of the timing logger. - const char* name_; + const char* const name_; // Do we want to print the exactly recorded split (true) or round down to the time unit being // used (false). @@ -162,7 +157,6 @@ class TimingLogger { DISALLOW_COPY_AND_ASSIGN(TimingLogger); }; -} // namespace base } // namespace art #endif // ART_RUNTIME_BASE_TIMING_LOGGER_H_ diff --git a/runtime/base/timing_logger_test.cc b/runtime/base/timing_logger_test.cc index 8f28e4809b..03cc9cc5e4 100644 --- a/runtime/base/timing_logger_test.cc +++ b/runtime/base/timing_logger_test.cc @@ -26,13 +26,13 @@ class TimingLoggerTest : public CommonTest {}; TEST_F(TimingLoggerTest, StartEnd) { const char* split1name = "First Split"; - base::TimingLogger timings("StartEnd", true, false); + TimingLogger timings("StartEnd", true, false); timings.StartSplit(split1name); timings.EndSplit(); // Ends split1. - const base::TimingLogger::SplitTimings& splits = timings.GetSplits(); + const TimingLogger::SplitTimings& splits = timings.GetSplits(); EXPECT_EQ(1U, splits.size()); EXPECT_STREQ(splits[0].second, split1name); @@ -43,7 +43,7 @@ TEST_F(TimingLoggerTest, StartNewEnd) { const char* split1name = "First Split"; const char* split2name = "Second Split"; const char* split3name = "Third Split"; - base::TimingLogger timings("StartNewEnd", true, false); + TimingLogger timings("StartNewEnd", true, false); timings.StartSplit(split1name); @@ -53,7 +53,7 @@ TEST_F(TimingLoggerTest, StartNewEnd) { timings.EndSplit(); // Ends split3. - const base::TimingLogger::SplitTimings& splits = timings.GetSplits(); + const TimingLogger::SplitTimings& splits = timings.GetSplits(); EXPECT_EQ(3U, splits.size()); EXPECT_STREQ(splits[0].second, split1name); @@ -67,7 +67,7 @@ TEST_F(TimingLoggerTest, StartNewEndNested) { const char* split3name = "Third Split"; const char* split4name = "Fourth Split"; const char* split5name = "Fifth Split"; - base::TimingLogger timings("StartNewEndNested", true, false); + TimingLogger timings("StartNewEndNested", true, false); timings.StartSplit(split1name); @@ -85,7 +85,7 @@ TEST_F(TimingLoggerTest, StartNewEndNested) { timings.EndSplit(); // Ends split2. - const base::TimingLogger::SplitTimings& splits = timings.GetSplits(); + const TimingLogger::SplitTimings& splits = timings.GetSplits(); EXPECT_EQ(5U, splits.size()); EXPECT_STREQ(splits[0].second, split1name); @@ -101,25 +101,25 @@ TEST_F(TimingLoggerTest, Scoped) { const char* innersplit1 = "Inner Split 1"; const char* innerinnersplit1 = "Inner Inner Split 1"; const char* innersplit2 = "Inner Split 2"; - base::TimingLogger timings("Scoped", true, false); + TimingLogger timings("Scoped", true, false); { - base::TimingLogger::ScopedSplit outer(outersplit, &timings); + TimingLogger::ScopedSplit outer(outersplit, &timings); { - base::TimingLogger::ScopedSplit inner1(innersplit1, &timings); + TimingLogger::ScopedSplit inner1(innersplit1, &timings); { - base::TimingLogger::ScopedSplit innerinner1(innerinnersplit1, &timings); + TimingLogger::ScopedSplit innerinner1(innerinnersplit1, &timings); } // Ends innerinnersplit1. } // Ends innersplit1. { - base::TimingLogger::ScopedSplit inner2(innersplit2, &timings); + TimingLogger::ScopedSplit inner2(innersplit2, &timings); } // Ends innersplit2. } // Ends outersplit. - const base::TimingLogger::SplitTimings& splits = timings.GetSplits(); + const TimingLogger::SplitTimings& splits = timings.GetSplits(); EXPECT_EQ(4U, splits.size()); EXPECT_STREQ(splits[0].second, innerinnersplit1); @@ -134,12 +134,12 @@ TEST_F(TimingLoggerTest, ScopedAndExplicit) { const char* innersplit = "Inner Split"; const char* innerinnersplit1 = "Inner Inner Split 1"; const char* innerinnersplit2 = "Inner Inner Split 2"; - base::TimingLogger timings("Scoped", true, false); + TimingLogger timings("Scoped", true, false); timings.StartSplit(outersplit); { - base::TimingLogger::ScopedSplit inner(innersplit, &timings); + TimingLogger::ScopedSplit inner(innersplit, &timings); timings.StartSplit(innerinnersplit1); @@ -148,7 +148,7 @@ TEST_F(TimingLoggerTest, ScopedAndExplicit) { timings.EndSplit(); // Ends outersplit. - const base::TimingLogger::SplitTimings& splits = timings.GetSplits(); + const TimingLogger::SplitTimings& splits = timings.GetSplits(); EXPECT_EQ(4U, splits.size()); EXPECT_STREQ(splits[0].second, innerinnersplit1); diff --git a/runtime/common_test.h b/runtime/common_test.h index 7cc29a1e58..d860b6c34a 100644 --- a/runtime/common_test.h +++ b/runtime/common_test.h @@ -582,10 +582,11 @@ class CommonTest : public testing::Test { void CompileMethod(mirror::ArtMethod* method) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { CHECK(method != NULL); - base::TimingLogger timings("CommonTest::CompileMethod", false, false); + TimingLogger timings("CommonTest::CompileMethod", false, false); timings.StartSplit("CompileOne"); compiler_driver_->CompileOne(method, timings); MakeExecutable(method); + timings.EndSplit(); } void CompileDirectMethod(SirtRef& class_loader, const char* class_name, diff --git a/runtime/gc/collector/garbage_collector.h b/runtime/gc/collector/garbage_collector.h index 6111c2fbf2..a80f5935c2 100644 --- a/runtime/gc/collector/garbage_collector.h +++ b/runtime/gc/collector/garbage_collector.h @@ -64,7 +64,7 @@ class GarbageCollector { void RegisterPause(uint64_t nano_length); - base::TimingLogger& GetTimings() { + TimingLogger& GetTimings() { return timings_; } @@ -131,7 +131,7 @@ class GarbageCollector { const bool verbose_; uint64_t duration_ns_; - base::TimingLogger timings_; + TimingLogger timings_; // Cumulative statistics. uint64_t total_time_ns_; diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc index 56dc0e528a..fc41c7fcc5 100644 --- a/runtime/gc/collector/mark_sweep.cc +++ b/runtime/gc/collector/mark_sweep.cc @@ -157,7 +157,7 @@ MarkSweep::MarkSweep(Heap* heap, bool is_concurrent, const std::string& name_pre void MarkSweep::InitializePhase() { timings_.Reset(); - base::TimingLogger::ScopedSplit split("InitializePhase", &timings_); + TimingLogger::ScopedSplit split("InitializePhase", &timings_); mark_stack_ = heap_->mark_stack_.get(); DCHECK(mark_stack_ != nullptr); SetImmuneRange(nullptr, nullptr); @@ -185,14 +185,14 @@ void MarkSweep::InitializePhase() { } void MarkSweep::ProcessReferences(Thread* self) { - base::TimingLogger::ScopedSplit split("ProcessReferences", &timings_); + TimingLogger::ScopedSplit split("ProcessReferences", &timings_); WriterMutexLock mu(self, *Locks::heap_bitmap_lock_); ProcessReferences(&soft_reference_list_, clear_soft_references_, &weak_reference_list_, &finalizer_reference_list_, &phantom_reference_list_); } bool MarkSweep::HandleDirtyObjectsPhase() { - base::TimingLogger::ScopedSplit split("HandleDirtyObjectsPhase", &timings_); + TimingLogger::ScopedSplit split("HandleDirtyObjectsPhase", &timings_); Thread* self = Thread::Current(); Locks::mutator_lock_->AssertExclusiveHeld(self); @@ -238,7 +238,7 @@ bool MarkSweep::IsConcurrent() const { } void MarkSweep::MarkingPhase() { - base::TimingLogger::ScopedSplit split("MarkingPhase", &timings_); + TimingLogger::ScopedSplit split("MarkingPhase", &timings_); Thread* self = Thread::Current(); BindBitmaps(); @@ -272,7 +272,7 @@ void MarkSweep::UpdateAndMarkModUnion() { if (IsImmuneSpace(space)) { const char* name = space->IsZygoteSpace() ? "UpdateAndMarkZygoteModUnionTable" : "UpdateAndMarkImageModUnionTable"; - base::TimingLogger::ScopedSplit split(name, &timings_); + TimingLogger::ScopedSplit split(name, &timings_); accounting::ModUnionTable* mod_union_table = heap_->FindModUnionTableFromSpace(space); CHECK(mod_union_table != nullptr); mod_union_table->UpdateAndMarkReferences(MarkRootCallback, this); @@ -297,7 +297,7 @@ void MarkSweep::MarkReachableObjects() { } void MarkSweep::ReclaimPhase() { - base::TimingLogger::ScopedSplit split("ReclaimPhase", &timings_); + TimingLogger::ScopedSplit split("ReclaimPhase", &timings_); Thread* self = Thread::Current(); if (!IsConcurrent()) { @@ -312,7 +312,7 @@ void MarkSweep::ReclaimPhase() { if (IsConcurrent()) { Runtime::Current()->AllowNewSystemWeaks(); - base::TimingLogger::ScopedSplit split("UnMarkAllocStack", &timings_); + TimingLogger::ScopedSplit split("UnMarkAllocStack", &timings_); WriterMutexLock mu(self, *Locks::heap_bitmap_lock_); accounting::ObjectStack* allocation_stack = GetHeap()->allocation_stack_.get(); // The allocation stack contains things allocated since the start of the GC. These may have been @@ -363,7 +363,7 @@ void MarkSweep::SetImmuneRange(Object* begin, Object* end) { } void MarkSweep::FindDefaultMarkBitmap() { - base::TimingLogger::ScopedSplit split("FindDefaultMarkBitmap", &timings_); + TimingLogger::ScopedSplit split("FindDefaultMarkBitmap", &timings_); for (const auto& space : GetHeap()->GetContinuousSpaces()) { accounting::SpaceBitmap* bitmap = space->GetMarkBitmap(); if (bitmap != nullptr && @@ -932,7 +932,7 @@ class RecursiveMarkTask : public MarkStackTask { // Populates the mark stack based on the set of marked objects and // recursively marks until the mark stack is emptied. void MarkSweep::RecursiveMark() { - base::TimingLogger::ScopedSplit split("RecursiveMark", &timings_); + TimingLogger::ScopedSplit split("RecursiveMark", &timings_); // RecursiveMark will build the lists of known instances of the Reference classes. // See DelayReferenceReferent for details. CHECK(soft_reference_list_ == NULL); @@ -1198,7 +1198,7 @@ void MarkSweep::SweepArray(accounting::ObjectStack* allocations, bool swap_bitma void MarkSweep::Sweep(bool swap_bitmaps) { DCHECK(mark_stack_->IsEmpty()); - base::TimingLogger::ScopedSplit("Sweep", &timings_); + TimingLogger::ScopedSplit("Sweep", &timings_); const bool partial = (GetGcType() == kGcTypePartial); SweepCallbackContext scc; @@ -1224,12 +1224,12 @@ void MarkSweep::Sweep(bool swap_bitmaps) { std::swap(live_bitmap, mark_bitmap); } if (!space->IsZygoteSpace()) { - base::TimingLogger::ScopedSplit split("SweepAllocSpace", &timings_); + TimingLogger::ScopedSplit split("SweepAllocSpace", &timings_); // Bitmaps are pre-swapped for optimization which enables sweeping with the heap unlocked. accounting::SpaceBitmap::SweepWalk(*live_bitmap, *mark_bitmap, begin, end, &SweepCallback, reinterpret_cast(&scc)); } else { - base::TimingLogger::ScopedSplit split("SweepZygote", &timings_); + TimingLogger::ScopedSplit split("SweepZygote", &timings_); // Zygote sweep takes care of dirtying cards and clearing live bits, does not free actual // memory. accounting::SpaceBitmap::SweepWalk(*live_bitmap, *mark_bitmap, begin, end, @@ -1242,7 +1242,7 @@ void MarkSweep::Sweep(bool swap_bitmaps) { } void MarkSweep::SweepLargeObjects(bool swap_bitmaps) { - base::TimingLogger::ScopedSplit("SweepLargeObjects", &timings_); + TimingLogger::ScopedSplit("SweepLargeObjects", &timings_); // Sweep large objects space::LargeObjectSpace* large_object_space = GetHeap()->GetLargeObjectsSpace(); accounting::SpaceSetMap* large_live_objects = large_object_space->GetLiveObjects(); @@ -1577,7 +1577,7 @@ void MarkSweep::ProcessReferences(Object** soft_references, bool clear_soft, } void MarkSweep::UnBindBitmaps() { - base::TimingLogger::ScopedSplit split("UnBindBitmaps", &timings_); + TimingLogger::ScopedSplit split("UnBindBitmaps", &timings_); for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->IsDlMallocSpace()) { space::DlMallocSpace* alloc_space = space->AsDlMallocSpace(); @@ -1594,7 +1594,7 @@ void MarkSweep::UnBindBitmaps() { } void MarkSweep::FinishPhase() { - base::TimingLogger::ScopedSplit split("FinishPhase", &timings_); + TimingLogger::ScopedSplit split("FinishPhase", &timings_); // Can't enqueue references if we hold the mutator lock. Object* cleared_references = GetClearedReferences(); Heap* heap = GetHeap(); diff --git a/runtime/gc/collector/semi_space.cc b/runtime/gc/collector/semi_space.cc index d833631da9..aafe7a9188 100644 --- a/runtime/gc/collector/semi_space.cc +++ b/runtime/gc/collector/semi_space.cc @@ -139,7 +139,7 @@ SemiSpace::SemiSpace(Heap* heap, const std::string& name_prefix) void SemiSpace::InitializePhase() { timings_.Reset(); - base::TimingLogger::ScopedSplit split("InitializePhase", &timings_); + TimingLogger::ScopedSplit split("InitializePhase", &timings_); mark_stack_ = heap_->mark_stack_.get(); DCHECK(mark_stack_ != nullptr); immune_begin_ = nullptr; @@ -156,7 +156,7 @@ void SemiSpace::InitializePhase() { } void SemiSpace::ProcessReferences(Thread* self) { - base::TimingLogger::ScopedSplit split("ProcessReferences", &timings_); + TimingLogger::ScopedSplit split("ProcessReferences", &timings_); WriterMutexLock mu(self, *Locks::heap_bitmap_lock_); ProcessReferences(&soft_reference_list_, clear_soft_references_, &weak_reference_list_, &finalizer_reference_list_, &phantom_reference_list_); @@ -165,7 +165,7 @@ void SemiSpace::ProcessReferences(Thread* self) { void SemiSpace::MarkingPhase() { Thread* self = Thread::Current(); Locks::mutator_lock_->AssertExclusiveHeld(self); - base::TimingLogger::ScopedSplit split("MarkingPhase", &timings_); + TimingLogger::ScopedSplit split("MarkingPhase", &timings_); // Need to do this with mutators paused so that somebody doesn't accidentally allocate into the // wrong space. heap_->SwapSemiSpaces(); @@ -198,7 +198,7 @@ void SemiSpace::UpdateAndMarkModUnion() { accounting::ModUnionTable* table = heap_->FindModUnionTableFromSpace(space); CHECK(table != nullptr); // TODO: Improve naming. - base::TimingLogger::ScopedSplit split( + TimingLogger::ScopedSplit split( space->IsZygoteSpace() ? "UpdateAndMarkZygoteModUnionTable" : "UpdateAndMarkImageModUnionTable", &timings_); @@ -218,7 +218,7 @@ void SemiSpace::MarkReachableObjects() { } void SemiSpace::ReclaimPhase() { - base::TimingLogger::ScopedSplit split("ReclaimPhase", &timings_); + TimingLogger::ScopedSplit split("ReclaimPhase", &timings_); Thread* self = Thread::Current(); ProcessReferences(self); { @@ -417,7 +417,7 @@ void SemiSpace::ZygoteSweepCallback(size_t num_ptrs, Object** ptrs, void* arg) { void SemiSpace::Sweep(bool swap_bitmaps) { DCHECK(mark_stack_->IsEmpty()); - base::TimingLogger::ScopedSplit("Sweep", &timings_); + TimingLogger::ScopedSplit("Sweep", &timings_); const bool partial = (GetGcType() == kGcTypePartial); SweepCallbackContext scc; @@ -443,12 +443,12 @@ void SemiSpace::Sweep(bool swap_bitmaps) { std::swap(live_bitmap, mark_bitmap); } if (!space->IsZygoteSpace()) { - base::TimingLogger::ScopedSplit split("SweepAllocSpace", &timings_); + TimingLogger::ScopedSplit split("SweepAllocSpace", &timings_); // Bitmaps are pre-swapped for optimization which enables sweeping with the heap unlocked. accounting::SpaceBitmap::SweepWalk(*live_bitmap, *mark_bitmap, begin, end, &SweepCallback, reinterpret_cast(&scc)); } else { - base::TimingLogger::ScopedSplit split("SweepZygote", &timings_); + TimingLogger::ScopedSplit split("SweepZygote", &timings_); // Zygote sweep takes care of dirtying cards and clearing live bits, does not free actual // memory. accounting::SpaceBitmap::SweepWalk(*live_bitmap, *mark_bitmap, begin, end, @@ -461,7 +461,7 @@ void SemiSpace::Sweep(bool swap_bitmaps) { } void SemiSpace::SweepLargeObjects(bool swap_bitmaps) { - base::TimingLogger::ScopedSplit("SweepLargeObjects", &timings_); + TimingLogger::ScopedSplit("SweepLargeObjects", &timings_); // Sweep large objects space::LargeObjectSpace* large_object_space = GetHeap()->GetLargeObjectsSpace(); accounting::SpaceSetMap* large_live_objects = large_object_space->GetLiveObjects(); @@ -725,7 +725,7 @@ void SemiSpace::ProcessReferences(Object** soft_references, bool clear_soft, } void SemiSpace::UnBindBitmaps() { - base::TimingLogger::ScopedSplit split("UnBindBitmaps", &timings_); + TimingLogger::ScopedSplit split("UnBindBitmaps", &timings_); for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->IsDlMallocSpace()) { space::DlMallocSpace* alloc_space = space->AsDlMallocSpace(); @@ -749,7 +749,7 @@ void SemiSpace::SetFromSpace(space::ContinuousMemMapAllocSpace* from_space) { } void SemiSpace::FinishPhase() { - base::TimingLogger::ScopedSplit split("FinishPhase", &timings_); + TimingLogger::ScopedSplit split("FinishPhase", &timings_); // Can't enqueue references if we hold the mutator lock. Object* cleared_references = GetClearedReferences(); Heap* heap = GetHeap(); diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc index 70df3d3133..c2094795b8 100644 --- a/runtime/gc/heap.cc +++ b/runtime/gc/heap.cc @@ -1474,7 +1474,7 @@ collector::GcType Heap::CollectGarbageInternal(collector::GcType gc_type, GcCaus << PrettySize(total_memory) << ", " << "paused " << pause_string.str() << " total " << PrettyDuration((duration / 1000) * 1000); if (VLOG_IS_ON(heap)) { - LOG(INFO) << Dumpable(collector->GetTimings()); + LOG(INFO) << Dumpable(collector->GetTimings()); } } } @@ -1808,17 +1808,17 @@ accounting::ModUnionTable* Heap::FindModUnionTableFromSpace(space::Space* space) return it->second; } -void Heap::ProcessCards(base::TimingLogger& timings) { +void Heap::ProcessCards(TimingLogger& timings) { // Clear cards and keep track of cards cleared in the mod-union table. for (const auto& space : continuous_spaces_) { accounting::ModUnionTable* table = FindModUnionTableFromSpace(space); if (table != nullptr) { const char* name = space->IsZygoteSpace() ? "ZygoteModUnionClearCards" : "ImageModUnionClearCards"; - base::TimingLogger::ScopedSplit split(name, &timings); + TimingLogger::ScopedSplit split(name, &timings); table->ClearCards(); } else if (space->GetType() != space::kSpaceTypeBumpPointerSpace) { - base::TimingLogger::ScopedSplit split("AllocSpaceClearCards", &timings); + TimingLogger::ScopedSplit split("AllocSpaceClearCards", &timings); // No mod union table for the AllocSpace. Age the cards so that the GC knows that these cards // were dirty before the GC started. // TODO: Don't need to use atomic. diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h index 0fa000f18d..52343bcf79 100644 --- a/runtime/gc/heap.h +++ b/runtime/gc/heap.h @@ -638,7 +638,7 @@ class Heap { void SwapStacks(); // Clear cards and update the mod union table. - void ProcessCards(base::TimingLogger& timings); + void ProcessCards(TimingLogger& timings); // All-known continuous spaces, where objects lie within fixed bounds. std::vector continuous_spaces_; -- cgit v1.2.3-59-g8ed1b From e9c36b34efb7460f59c6766e526c9b0de8da70b3 Mon Sep 17 00:00:00 2001 From: Vladimir Marko Date: Thu, 21 Nov 2013 15:49:16 +0000 Subject: Avoid some string allocations. Also avoid building a string one character at a time. Change-Id: I3db26226c620a730b95637d5bfc23e2d4715cfb9 --- compiler/driver/compiler_driver.cc | 4 ++-- runtime/dex_file.cc | 8 +++----- runtime/hprof/hprof.cc | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) (limited to 'compiler/driver/compiler_driver.cc') diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index b9df1d6f48..7b428793ab 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -697,11 +697,11 @@ void CompilerDriver::LoadImageClasses(TimingLogger& timings) ScopedObjectAccess soa(self); ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); for (auto it = image_classes_->begin(), end = image_classes_->end(); it != end;) { - std::string descriptor(*it); + const std::string& descriptor(*it); SirtRef klass(self, class_linker->FindSystemClass(descriptor.c_str())); if (klass.get() == NULL) { - image_classes_->erase(it++); VLOG(compiler) << "Failed to find class " << descriptor; + image_classes_->erase(it++); self->ClearException(); } else { ++it; diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc index 0ddcdf30c9..a7575ce889 100644 --- a/runtime/dex_file.cc +++ b/runtime/dex_file.cc @@ -555,22 +555,19 @@ bool DexFile::CreateTypeList(const StringPiece& signature, uint16_t* return_type size_t end = signature.size(); bool process_return = false; while (offset < end) { + size_t start_offset = offset; char c = signature[offset]; offset++; if (c == ')') { process_return = true; continue; } - // TODO: avoid building a string. - std::string descriptor; - descriptor += c; while (c == '[') { // process array prefix if (offset >= end) { // expect some descriptor following [ return false; } c = signature[offset]; offset++; - descriptor += c; } if (c == 'L') { // process type descriptors do { @@ -579,9 +576,10 @@ bool DexFile::CreateTypeList(const StringPiece& signature, uint16_t* return_type } c = signature[offset]; offset++; - descriptor += c; } while (c != ';'); } + // TODO: avoid creating a std::string just to get a 0-terminated char array + std::string descriptor(signature.data() + start_offset, offset - start_offset); const DexFile::StringId* string_id = FindStringId(descriptor.c_str()); if (string_id == NULL) { return false; diff --git a/runtime/hprof/hprof.cc b/runtime/hprof/hprof.cc index 67620a09f1..9f899e87a3 100644 --- a/runtime/hprof/hprof.cc +++ b/runtime/hprof/hprof.cc @@ -537,7 +537,7 @@ class Hprof { HprofRecord* rec = ¤t_record_; for (StringMapIterator it = strings_.begin(); it != strings_.end(); ++it) { - std::string string((*it).first); + const std::string& string = (*it).first; size_t id = (*it).second; int err = current_record_.StartNewRecord(header_fp_, HPROF_TAG_STRING, HPROF_TIME); -- cgit v1.2.3-59-g8ed1b