OSDN Git Service

Clean up transaction
authorMathieu Chartier <mathieuc@google.com>
Sat, 4 Feb 2017 00:06:35 +0000 (16:06 -0800)
committerMathieu Chartier <mathieuc@google.com>
Sat, 4 Feb 2017 00:15:38 +0000 (16:15 -0800)
Disallow copy constructors, use move constructors, and fix
formatting. Aims to prevent future bugs.

Test: test-art-host

Bug: 34963782

Change-Id: Iaf626e94b14a1fd1b6b64fbd434f5de678e53dc2

runtime/transaction.cc
runtime/transaction.h

index f94bb65..56ff0a1 100644 (file)
@@ -41,12 +41,12 @@ Transaction::~Transaction() {
     MutexLock mu(Thread::Current(), log_lock_);
     size_t objects_count = object_logs_.size();
     size_t field_values_count = 0;
-    for (auto it : object_logs_) {
+    for (const auto& it : object_logs_) {
       field_values_count += it.second.Size();
     }
     size_t array_count = array_logs_.size();
     size_t array_values_count = 0;
-    for (auto it : array_logs_) {
+    for (const auto& it : array_logs_) {
       array_values_count += it.second.Size();
     }
     size_t intern_string_count = intern_string_logs_.size();
@@ -100,24 +100,30 @@ const std::string& Transaction::GetAbortMessage() {
   return abort_message_;
 }
 
-void Transaction::RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset,
-                                          uint8_t value, bool is_volatile) {
+void Transaction::RecordWriteFieldBoolean(mirror::Object* obj,
+                                          MemberOffset field_offset,
+                                          uint8_t value,
+                                          bool is_volatile) {
   DCHECK(obj != nullptr);
   MutexLock mu(Thread::Current(), log_lock_);
   ObjectLog& object_log = object_logs_[obj];
   object_log.LogBooleanValue(field_offset, value, is_volatile);
 }
 
-void Transaction::RecordWriteFieldByte(mirror::Object* obj, MemberOffset field_offset,
-                                       int8_t value, bool is_volatile) {
+void Transaction::RecordWriteFieldByte(mirror::Object* obj,
+                                       MemberOffset field_offset,
+                                       int8_t value,
+                                       bool is_volatile) {
   DCHECK(obj != nullptr);
   MutexLock mu(Thread::Current(), log_lock_);
   ObjectLog& object_log = object_logs_[obj];
   object_log.LogByteValue(field_offset, value, is_volatile);
 }
 
-void Transaction::RecordWriteFieldChar(mirror::Object* obj, MemberOffset field_offset,
-                                       uint16_t value, bool is_volatile) {
+void Transaction::RecordWriteFieldChar(mirror::Object* obj,
+                                       MemberOffset field_offset,
+                                       uint16_t value,
+                                       bool is_volatile) {
   DCHECK(obj != nullptr);
   MutexLock mu(Thread::Current(), log_lock_);
   ObjectLog& object_log = object_logs_[obj];
@@ -125,8 +131,10 @@ void Transaction::RecordWriteFieldChar(mirror::Object* obj, MemberOffset field_o
 }
 
 
-void Transaction::RecordWriteFieldShort(mirror::Object* obj, MemberOffset field_offset,
-                                        int16_t value, bool is_volatile) {
+void Transaction::RecordWriteFieldShort(mirror::Object* obj,
+                                        MemberOffset field_offset,
+                                        int16_t value,
+                                        bool is_volatile) {
   DCHECK(obj != nullptr);
   MutexLock mu(Thread::Current(), log_lock_);
   ObjectLog& object_log = object_logs_[obj];
@@ -134,7 +142,9 @@ void Transaction::RecordWriteFieldShort(mirror::Object* obj, MemberOffset field_
 }
 
 
-void Transaction::RecordWriteField32(mirror::Object* obj, MemberOffset field_offset, uint32_t value,
+void Transaction::RecordWriteField32(mirror::Object* obj,
+                                     MemberOffset field_offset,
+                                     uint32_t value,
                                      bool is_volatile) {
   DCHECK(obj != nullptr);
   MutexLock mu(Thread::Current(), log_lock_);
@@ -142,7 +152,9 @@ void Transaction::RecordWriteField32(mirror::Object* obj, MemberOffset field_off
   object_log.Log32BitsValue(field_offset, value, is_volatile);
 }
 
-void Transaction::RecordWriteField64(mirror::Object* obj, MemberOffset field_offset, uint64_t value,
+void Transaction::RecordWriteField64(mirror::Object* obj,
+                                     MemberOffset field_offset,
+                                     uint64_t value,
                                      bool is_volatile) {
   DCHECK(obj != nullptr);
   MutexLock mu(Thread::Current(), log_lock_);
@@ -150,8 +162,10 @@ void Transaction::RecordWriteField64(mirror::Object* obj, MemberOffset field_off
   object_log.Log64BitsValue(field_offset, value, is_volatile);
 }
 
-void Transaction::RecordWriteFieldReference(mirror::Object* obj, MemberOffset field_offset,
-                                            mirror::Object* value, bool is_volatile) {
+void Transaction::RecordWriteFieldReference(mirror::Object* obj,
+                                            MemberOffset field_offset,
+                                            mirror::Object* value,
+                                            bool is_volatile) {
   DCHECK(obj != nullptr);
   MutexLock mu(Thread::Current(), log_lock_);
   ObjectLog& object_log = object_logs_[obj];
@@ -163,8 +177,12 @@ void Transaction::RecordWriteArray(mirror::Array* array, size_t index, uint64_t
   DCHECK(array->IsArrayInstance());
   DCHECK(!array->IsObjectArray());
   MutexLock mu(Thread::Current(), log_lock_);
-  ArrayLog& array_log = array_logs_[array];
-  array_log.LogValue(index, value);
+  auto it = array_logs_.find(array);
+  if (it == array_logs_.end()) {
+    ArrayLog log;
+    it = array_logs_.emplace(array, std::move(log)).first;
+  }
+  it->second.LogValue(index, value);
 }
 
 void Transaction::RecordResolveString(ObjPtr<mirror::DexCache> dex_cache,
@@ -172,33 +190,33 @@ void Transaction::RecordResolveString(ObjPtr<mirror::DexCache> dex_cache,
   DCHECK(dex_cache != nullptr);
   DCHECK_LT(string_idx.index_, dex_cache->GetDexFile()->NumStringIds());
   MutexLock mu(Thread::Current(), log_lock_);
-  resolve_string_logs_.push_back(ResolveStringLog(dex_cache, string_idx));
+  resolve_string_logs_.emplace_back(dex_cache, string_idx);
 }
 
 void Transaction::RecordStrongStringInsertion(ObjPtr<mirror::String> s) {
   InternStringLog log(s, InternStringLog::kStrongString, InternStringLog::kInsert);
-  LogInternedString(log);
+  LogInternedString(std::move(log));
 }
 
 void Transaction::RecordWeakStringInsertion(ObjPtr<mirror::String> s) {
   InternStringLog log(s, InternStringLog::kWeakString, InternStringLog::kInsert);
-  LogInternedString(log);
+  LogInternedString(std::move(log));
 }
 
 void Transaction::RecordStrongStringRemoval(ObjPtr<mirror::String> s) {
   InternStringLog log(s, InternStringLog::kStrongString, InternStringLog::kRemove);
-  LogInternedString(log);
+  LogInternedString(std::move(log));
 }
 
 void Transaction::RecordWeakStringRemoval(ObjPtr<mirror::String> s) {
   InternStringLog log(s, InternStringLog::kWeakString, InternStringLog::kRemove);
-  LogInternedString(log);
+  LogInternedString(std::move(log));
 }
 
-void Transaction::LogInternedString(const InternStringLog& log) {
+void Transaction::LogInternedString(InternStringLog&& log) {
   Locks::intern_table_lock_->AssertExclusiveHeld(Thread::Current());
   MutexLock mu(Thread::Current(), log_lock_);
-  intern_string_logs_.push_front(log);
+  intern_string_logs_.push_front(std::move(log));
 }
 
 void Transaction::Rollback() {
@@ -216,7 +234,7 @@ void Transaction::Rollback() {
 void Transaction::UndoObjectModifications() {
   // TODO we may not need to restore objects allocated during this transaction. Or we could directly
   // remove them from the heap.
-  for (auto it : object_logs_) {
+  for (const auto& it : object_logs_) {
     it.second.Undo(it.first);
   }
   object_logs_.clear();
@@ -225,7 +243,7 @@ void Transaction::UndoObjectModifications() {
 void Transaction::UndoArrayModifications() {
   // TODO we may not need to restore array allocated during this transaction. Or we could directly
   // remove them from the heap.
-  for (auto it : array_logs_) {
+  for (const auto& it : array_logs_) {
     it.second.Undo(it.first);
   }
   array_logs_.clear();
@@ -235,7 +253,7 @@ void Transaction::UndoInternStringTableModifications() {
   InternTable* const intern_table = Runtime::Current()->GetInternTable();
   // We want to undo each operation from the most recent to the oldest. List has been filled so the
   // most recent operation is at list begin so just have to iterate over it.
-  for (InternStringLog& string_log : intern_string_logs_) {
+  for (const InternStringLog& string_log : intern_string_logs_) {
     string_log.Undo(intern_table);
   }
   intern_string_logs_.clear();
@@ -279,7 +297,7 @@ void Transaction::VisitObjectLogs(RootVisitor* visitor) {
     auto old_root_it = object_logs_.find(old_root);
     CHECK(old_root_it != object_logs_.end());
     CHECK(object_logs_.find(new_root) == object_logs_.end());
-    object_logs_.insert(std::make_pair(new_root, old_root_it->second));
+    object_logs_.emplace(new_root, std::move(old_root_it->second));
     object_logs_.erase(old_root_it);
   }
 }
@@ -306,7 +324,7 @@ void Transaction::VisitArrayLogs(RootVisitor* visitor) {
     auto old_root_it = array_logs_.find(old_root);
     CHECK(old_root_it != array_logs_.end());
     CHECK(array_logs_.find(new_root) == array_logs_.end());
-    array_logs_.insert(std::make_pair(new_root, old_root_it->second));
+    array_logs_.emplace(new_root, std::move(old_root_it->second));
     array_logs_.erase(old_root_it);
   }
 }
@@ -347,23 +365,27 @@ void Transaction::ObjectLog::Log64BitsValue(MemberOffset offset, uint64_t value,
   LogValue(ObjectLog::k64Bits, offset, value, is_volatile);
 }
 
-void Transaction::ObjectLog::LogReferenceValue(MemberOffset offset, mirror::Object* obj, bool is_volatile) {
+void Transaction::ObjectLog::LogReferenceValue(MemberOffset offset,
+                                               mirror::Object* obj,
+                                               bool is_volatile) {
   LogValue(ObjectLog::kReference, offset, reinterpret_cast<uintptr_t>(obj), is_volatile);
 }
 
 void Transaction::ObjectLog::LogValue(ObjectLog::FieldValueKind kind,
-                                      MemberOffset offset, uint64_t value, bool is_volatile) {
+                                      MemberOffset offset,
+                                      uint64_t value,
+                                      bool is_volatile) {
   auto it = field_values_.find(offset.Uint32Value());
   if (it == field_values_.end()) {
     ObjectLog::FieldValue field_value;
     field_value.value = value;
     field_value.is_volatile = is_volatile;
     field_value.kind = kind;
-    field_values_.insert(std::make_pair(offset.Uint32Value(), field_value));
+    field_values_.emplace(offset.Uint32Value(), std::move(field_value));
   }
 }
 
-void Transaction::ObjectLog::Undo(mirror::Object* obj) {
+void Transaction::ObjectLog::Undo(mirror::Object* obj) const {
   for (auto& it : field_values_) {
     // Garbage collector needs to access object's class and array's length. So we don't rollback
     // these values.
@@ -377,60 +399,71 @@ void Transaction::ObjectLog::Undo(mirror::Object* obj) {
       // Skip Array::length field.
       continue;
     }
-    FieldValue& field_value = it.second;
+    const FieldValue& field_value = it.second;
     UndoFieldWrite(obj, field_offset, field_value);
   }
 }
 
-void Transaction::ObjectLog::UndoFieldWrite(mirror::Object* obj, MemberOffset field_offset,
-                                            const FieldValue& field_value) {
+void Transaction::ObjectLog::UndoFieldWrite(mirror::Object* obj,
+                                            MemberOffset field_offset,
+                                            const FieldValue& field_value) const {
   // TODO We may want to abort a transaction while still being in transaction mode. In this case,
   // we'd need to disable the check.
   constexpr bool kCheckTransaction = true;
   switch (field_value.kind) {
     case kBoolean:
       if (UNLIKELY(field_value.is_volatile)) {
-        obj->SetFieldBooleanVolatile<false, kCheckTransaction>(field_offset,
-                                                         static_cast<bool>(field_value.value));
+        obj->SetFieldBooleanVolatile<false, kCheckTransaction>(
+            field_offset,
+            static_cast<bool>(field_value.value));
       } else {
-        obj->SetFieldBoolean<false, kCheckTransaction>(field_offset,
-                                                 static_cast<bool>(field_value.value));
+        obj->SetFieldBoolean<false, kCheckTransaction>(
+            field_offset,
+            static_cast<bool>(field_value.value));
       }
       break;
     case kByte:
       if (UNLIKELY(field_value.is_volatile)) {
-        obj->SetFieldByteVolatile<false, kCheckTransaction>(field_offset,
-                                                         static_cast<int8_t>(field_value.value));
+        obj->SetFieldByteVolatile<false, kCheckTransaction>(
+            field_offset,
+            static_cast<int8_t>(field_value.value));
       } else {
-        obj->SetFieldByte<false, kCheckTransaction>(field_offset,
-                                                 static_cast<int8_t>(field_value.value));
+        obj->SetFieldByte<false, kCheckTransaction>(
+            field_offset,
+            static_cast<int8_t>(field_value.value));
       }
       break;
     case kChar:
       if (UNLIKELY(field_value.is_volatile)) {
-        obj->SetFieldCharVolatile<false, kCheckTransaction>(field_offset,
-                                                          static_cast<uint16_t>(field_value.value));
+        obj->SetFieldCharVolatile<false, kCheckTransaction>(
+            field_offset,
+            static_cast<uint16_t>(field_value.value));
       } else {
-        obj->SetFieldChar<false, kCheckTransaction>(field_offset,
-                                                  static_cast<uint16_t>(field_value.value));
+        obj->SetFieldChar<false, kCheckTransaction>(
+            field_offset,
+            static_cast<uint16_t>(field_value.value));
       }
       break;
     case kShort:
       if (UNLIKELY(field_value.is_volatile)) {
-        obj->SetFieldShortVolatile<false, kCheckTransaction>(field_offset,
-                                                          static_cast<int16_t>(field_value.value));
+        obj->SetFieldShortVolatile<false, kCheckTransaction>(
+            field_offset,
+            static_cast<int16_t>(field_value.value));
       } else {
-        obj->SetFieldShort<false, kCheckTransaction>(field_offset,
-                                                  static_cast<int16_t>(field_value.value));
+        obj->SetFieldShort<false, kCheckTransaction>(
+            field_offset,
+            static_cast<int16_t>(field_value.value));
       }
       break;
     case k32Bits:
       if (UNLIKELY(field_value.is_volatile)) {
-        obj->SetField32Volatile<false, kCheckTransaction>(field_offset,
-                                                          static_cast<uint32_t>(field_value.value));
+        obj->SetField32Volatile<false, kCheckTransaction>(
+            field_offset,
+            static_cast<uint32_t>(field_value.value));
       } else {
-        obj->SetField32<false, kCheckTransaction>(field_offset,
-                                                  static_cast<uint32_t>(field_value.value));
+        obj->SetField32<false, kCheckTransaction>(
+            field_offset,
+            static_cast<uint32_t>(field_value.value));
       }
       break;
     case k64Bits:
@@ -442,11 +475,13 @@ void Transaction::ObjectLog::UndoFieldWrite(mirror::Object* obj, MemberOffset fi
       break;
     case kReference:
       if (UNLIKELY(field_value.is_volatile)) {
-        obj->SetFieldObjectVolatile<false, kCheckTransaction>(field_offset,
-                                                              reinterpret_cast<mirror::Object*>(field_value.value));
+        obj->SetFieldObjectVolatile<false, kCheckTransaction>(
+            field_offset,
+            reinterpret_cast<mirror::Object*>(field_value.value));
       } else {
-        obj->SetFieldObject<false, kCheckTransaction>(field_offset,
-                                                      reinterpret_cast<mirror::Object*>(field_value.value));
+        obj->SetFieldObject<false, kCheckTransaction>(
+            field_offset,
+            reinterpret_cast<mirror::Object*>(field_value.value));
       }
       break;
     default:
@@ -465,7 +500,7 @@ void Transaction::ObjectLog::VisitRoots(RootVisitor* visitor) {
   }
 }
 
-void Transaction::InternStringLog::Undo(InternTable* intern_table) {
+void Transaction::InternStringLog::Undo(InternTable* intern_table) const {
   DCHECK(intern_table != nullptr);
   switch (string_op_) {
     case InternStringLog::kInsert: {
@@ -506,7 +541,7 @@ void Transaction::InternStringLog::VisitRoots(RootVisitor* visitor) {
   str_.VisitRoot(visitor, RootInfo(kRootInternedString));
 }
 
-void Transaction::ResolveStringLog::Undo() {
+void Transaction::ResolveStringLog::Undo() const {
   dex_cache_.Read()->ClearString(string_idx_);
 }
 
@@ -538,7 +573,7 @@ void Transaction::ArrayLog::LogValue(size_t index, uint64_t value) {
   }
 }
 
-void Transaction::ArrayLog::Undo(mirror::Array* array) {
+void Transaction::ArrayLog::Undo(mirror::Array* array) const {
   DCHECK(array != nullptr);
   DCHECK(array->IsArrayInstance());
   Primitive::Type type = array->GetClass()->GetComponentType()->GetPrimitiveType();
@@ -547,8 +582,10 @@ void Transaction::ArrayLog::Undo(mirror::Array* array) {
   }
 }
 
-void Transaction::ArrayLog::UndoArrayWrite(mirror::Array* array, Primitive::Type array_type,
-                                           size_t index, uint64_t value) {
+void Transaction::ArrayLog::UndoArrayWrite(mirror::Array* array,
+                                           Primitive::Type array_type,
+                                           size_t index,
+                                           uint64_t value) const {
   // TODO We may want to abort a transaction while still being in transaction mode. In this case,
   // we'd need to disable the check.
   switch (array_type) {
index 1774657..7aa98cd 100644 (file)
@@ -56,26 +56,40 @@ class Transaction FINAL {
   bool IsAborted() REQUIRES(!log_lock_);
 
   // Record object field changes.
-  void RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset, uint8_t value,
+  void RecordWriteFieldBoolean(mirror::Object* obj,
+                               MemberOffset field_offset,
+                               uint8_t value,
                                bool is_volatile)
       REQUIRES(!log_lock_);
-  void RecordWriteFieldByte(mirror::Object* obj, MemberOffset field_offset, int8_t value,
-                               bool is_volatile)
+  void RecordWriteFieldByte(mirror::Object* obj,
+                            MemberOffset field_offset,
+                            int8_t value,
+                            bool is_volatile)
       REQUIRES(!log_lock_);
-  void RecordWriteFieldChar(mirror::Object* obj, MemberOffset field_offset, uint16_t value,
+  void RecordWriteFieldChar(mirror::Object* obj,
+                            MemberOffset field_offset,
+                            uint16_t value,
                             bool is_volatile)
       REQUIRES(!log_lock_);
-  void RecordWriteFieldShort(mirror::Object* obj, MemberOffset field_offset, int16_t value,
+  void RecordWriteFieldShort(mirror::Object* obj,
+                             MemberOffset field_offset,
+                             int16_t value,
                              bool is_volatile)
       REQUIRES(!log_lock_);
-  void RecordWriteField32(mirror::Object* obj, MemberOffset field_offset, uint32_t value,
+  void RecordWriteField32(mirror::Object* obj,
+                          MemberOffset field_offset,
+                          uint32_t value,
                           bool is_volatile)
       REQUIRES(!log_lock_);
-  void RecordWriteField64(mirror::Object* obj, MemberOffset field_offset, uint64_t value,
+  void RecordWriteField64(mirror::Object* obj,
+                          MemberOffset field_offset,
+                          uint64_t value,
                           bool is_volatile)
       REQUIRES(!log_lock_);
-  void RecordWriteFieldReference(mirror::Object* obj, MemberOffset field_offset,
-                                 mirror::Object* value, bool is_volatile)
+  void RecordWriteFieldReference(mirror::Object* obj,
+                                 MemberOffset field_offset,
+                                 mirror::Object* value,
+                                 bool is_volatile)
       REQUIRES(!log_lock_);
 
   // Record array change.
@@ -122,13 +136,16 @@ class Transaction FINAL {
     void Log64BitsValue(MemberOffset offset, uint64_t value, bool is_volatile);
     void LogReferenceValue(MemberOffset offset, mirror::Object* obj, bool is_volatile);
 
-    void Undo(mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_);
+    void Undo(mirror::Object* obj) const REQUIRES_SHARED(Locks::mutator_lock_);
     void VisitRoots(RootVisitor* visitor) REQUIRES_SHARED(Locks::mutator_lock_);
 
     size_t Size() const {
       return field_values_.size();
     }
 
+    ObjectLog() = default;
+    ObjectLog(ObjectLog&& log) = default;
+
    private:
     enum FieldValueKind {
       kBoolean,
@@ -144,33 +161,49 @@ class Transaction FINAL {
       uint64_t value;
       FieldValueKind kind;
       bool is_volatile;
+
+      FieldValue() = default;
+      FieldValue(FieldValue&& log) = default;
+
+     private:
+      DISALLOW_COPY_AND_ASSIGN(FieldValue);
     };
 
     void LogValue(FieldValueKind kind, MemberOffset offset, uint64_t value, bool is_volatile);
-    void UndoFieldWrite(mirror::Object* obj, MemberOffset field_offset,
-                        const FieldValue& field_value) REQUIRES_SHARED(Locks::mutator_lock_);
+    void UndoFieldWrite(mirror::Object* obj,
+                        MemberOffset field_offset,
+                        const FieldValue& field_value) const REQUIRES_SHARED(Locks::mutator_lock_);
 
     // Maps field's offset to its value.
     std::map<uint32_t, FieldValue> field_values_;
+
+    DISALLOW_COPY_AND_ASSIGN(ObjectLog);
   };
 
   class ArrayLog : public ValueObject {
    public:
     void LogValue(size_t index, uint64_t value);
 
-    void Undo(mirror::Array* obj) REQUIRES_SHARED(Locks::mutator_lock_);
+    void Undo(mirror::Array* obj) const REQUIRES_SHARED(Locks::mutator_lock_);
 
     size_t Size() const {
       return array_values_.size();
     }
 
+    ArrayLog() = default;
+    ArrayLog(ArrayLog&& log) = default;
+
    private:
-    void UndoArrayWrite(mirror::Array* array, Primitive::Type array_type, size_t index,
-                        uint64_t value) REQUIRES_SHARED(Locks::mutator_lock_);
+    void UndoArrayWrite(mirror::Array* array,
+                        Primitive::Type array_type,
+                        size_t index,
+                        uint64_t value) const REQUIRES_SHARED(Locks::mutator_lock_);
 
     // Maps index to value.
     // TODO use JValue instead ?
     std::map<size_t, uint64_t> array_values_;
+
+    DISALLOW_COPY_AND_ASSIGN(ArrayLog);
   };
 
   class InternStringLog : public ValueObject {
@@ -185,31 +218,38 @@ class Transaction FINAL {
     };
     InternStringLog(ObjPtr<mirror::String> s, StringKind kind, StringOp op);
 
-    void Undo(InternTable* intern_table)
+    void Undo(InternTable* intern_table) const
         REQUIRES_SHARED(Locks::mutator_lock_)
         REQUIRES(Locks::intern_table_lock_);
     void VisitRoots(RootVisitor* visitor) REQUIRES_SHARED(Locks::mutator_lock_);
 
+    InternStringLog() = default;
+    InternStringLog(InternStringLog&& log) = default;
+
    private:
-    GcRoot<mirror::String> str_;
+    mutable GcRoot<mirror::String> str_;
     const StringKind string_kind_;
     const StringOp string_op_;
+
+    DISALLOW_COPY_AND_ASSIGN(InternStringLog);
   };
 
   class ResolveStringLog : public ValueObject {
    public:
     ResolveStringLog(ObjPtr<mirror::DexCache> dex_cache, dex::StringIndex string_idx);
 
-    void Undo() REQUIRES_SHARED(Locks::mutator_lock_);
+    void Undo() const REQUIRES_SHARED(Locks::mutator_lock_);
 
     void VisitRoots(RootVisitor* visitor) REQUIRES_SHARED(Locks::mutator_lock_);
 
    private:
     GcRoot<mirror::DexCache> dex_cache_;
     const dex::StringIndex string_idx_;
+
+    DISALLOW_COPY_AND_ASSIGN(ResolveStringLog);
   };
 
-  void LogInternedString(const InternStringLog& log)
+  void LogInternedString(InternStringLog&& log)
       REQUIRES(Locks::intern_table_lock_)
       REQUIRES(!log_lock_);