Avoid transaction records for all new objects.

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing --interp-ac
Bug: 181943478
Change-Id: I0a51f622812dd3be0844c9e7c79e3b569ac3f029

5 files changed, 110 insertions, 32 deletions

diff --git a/runtime/interpreter/interpreter_switch_impl-inl.h b/runtime/interpreter/interpreter_switch_impl-inl.h
index 72a95953ad..1ebac52c1e 100644
--- a/runtime/interpreter/interpreter_switch_impl-inl.h
+++ b/runtime/interpreter/interpreter_switch_impl-inl.h
@@ -861,7 +861,7 @@ class InstructionHandler {
       } else {
         obj = AllocObjectFromCode(c, Self(), allocator_type);
         if (obj != nullptr) {
-          TransactionChecker::RecordAllocatedObject(obj);
+          TransactionChecker::RecordNewObject(obj);
         }
       }
     }
@@ -875,17 +875,17 @@ class InstructionHandler {
 
   HANDLER_ATTRIBUTES bool NEW_ARRAY() {
     int32_t length = GetVReg(B());
-    ObjPtr<mirror::Object> obj = AllocArrayFromCode(
+    ObjPtr<mirror::Array> array = AllocArrayFromCode(
         dex::TypeIndex(C()),
         length,
         shadow_frame_.GetMethod(),
         Self(),
         Runtime::Current()->GetHeap()->GetCurrentAllocator());
-    if (UNLIKELY(obj == nullptr)) {
+    if (UNLIKELY(array == nullptr)) {
       return false;  // Pending exception.
     }
-    TransactionChecker::RecordAllocatedObject(obj);
-    SetVRegReference(A(), obj);
+    TransactionChecker::RecordNewArray(array);
+    SetVRegReference(A(), array);
     return true;
   }
 
diff --git a/runtime/interpreter/interpreter_switch_impl0.cc b/runtime/interpreter/interpreter_switch_impl0.cc
index 53e37427b3..1bea9da838 100644
--- a/runtime/interpreter/interpreter_switch_impl0.cc
+++ b/runtime/interpreter/interpreter_switch_impl0.cc
@@ -57,8 +57,10 @@ class InactiveTransactionChecker {
                                                [[maybe_unused]] int32_t count)
       REQUIRES_SHARED(Locks::mutator_lock_) {}
 
-  ALWAYS_INLINE
-  static void RecordAllocatedObject([[maybe_unused]] ObjPtr<mirror::Object> new_object)
+  ALWAYS_INLINE static void RecordNewObject([[maybe_unused]] ObjPtr<mirror::Object> new_object)
+      REQUIRES_SHARED(Locks::mutator_lock_) {}
+
+  ALWAYS_INLINE static void RecordNewArray([[maybe_unused]] ObjPtr<mirror::Array> new_array)
       REQUIRES_SHARED(Locks::mutator_lock_) {}
 };
 
diff --git a/runtime/interpreter/interpreter_switch_impl1.cc b/runtime/interpreter/interpreter_switch_impl1.cc
index 71d77e2064..9c5b5b3266 100644
--- a/runtime/interpreter/interpreter_switch_impl1.cc
+++ b/runtime/interpreter/interpreter_switch_impl1.cc
@@ -54,9 +54,14 @@ class ActiveTransactionChecker {
   static void RecordArrayElementsInTransaction(ObjPtr<mirror::Object> array, int32_t count)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
-  static void RecordAllocatedObject([[maybe_unused]] ObjPtr<mirror::Object> new_object)
+  static void RecordNewObject(ObjPtr<mirror::Object> new_object)
       REQUIRES_SHARED(Locks::mutator_lock_) {
-    GetClassLinker()->GetTransaction()->RecordAllocatedObject(new_object);
+    GetClassLinker()->GetTransaction()->RecordNewObject(new_object);
+  }
+
+  static void RecordNewArray(ObjPtr<mirror::Array> new_object)
+      REQUIRES_SHARED(Locks::mutator_lock_) {
+    GetClassLinker()->GetTransaction()->RecordNewArray(new_object);
   }
 
  private:
@@ -85,7 +90,7 @@ void ActiveTransactionChecker::RecordArrayElementsInTransaction(ObjPtr<mirror::O
   DCHECK(array->IsArrayInstance());
   DCHECK_LE(count, array->AsArray()->GetLength());
   Transaction* transaction = GetClassLinker()->GetTransaction();
-  if (!transaction->NeedsTransactionRecords(array.Ptr())) {
+  if (!transaction->ArrayNeedsTransactionRecords(array->AsArray())) {
     return;
   }
   // No read barrier is needed for reading a chain of constant references
diff --git a/runtime/transaction.cc b/runtime/transaction.cc
index 3343f3a172..9b3985503a 100644
--- a/runtime/transaction.cc
+++ b/runtime/transaction.cc
@@ -165,6 +165,42 @@ bool Transaction::ReadConstraint(ObjPtr<mirror::Object> obj) const {
   }
 }
 
+void Transaction::RecordNewObject(ObjPtr<mirror::Object> obj) {
+  last_allocated_object_ = obj.Ptr();
+  ObjectLog log(&allocator_);
+  log.MarkAsNewObject();
+  object_logs_.Put(obj.Ptr(), std::move(log));
+}
+
+void Transaction::RecordNewArray(ObjPtr<mirror::Array> array) {
+  if (array->IsObjectArray()) {
+    // `ObjectArray<T>::SetWithoutChecks()` uses `SetFieldObject()` which records value
+    // changes in `object_log_`, so we need to record new object arrays as normal objects.
+    RecordNewObject(array);
+    return;
+  }
+  last_allocated_object_ = array.Ptr();
+  ArrayLog log(&allocator_);
+  log.MarkAsNewArray();
+  array_logs_.Put(array.Ptr(), std::move(log));
+}
+
+bool Transaction::ObjectNeedsTransactionRecords(ObjPtr<mirror::Object> obj) {
+  if (obj == last_allocated_object_) {
+    return false;
+  }
+  auto it = object_logs_.find(obj.Ptr());
+  return it == object_logs_.end() || !it->second.IsNewObject();
+}
+
+bool Transaction::ArrayNeedsTransactionRecords(ObjPtr<mirror::Array> array) {
+  if (array == last_allocated_object_) {
+    return false;
+  }
+  auto it = array_logs_.find(array.Ptr());
+  return it == array_logs_.end() || !it->second.IsNewArray();
+}
+
 inline Transaction::ObjectLog& Transaction::GetOrCreateObjectLog(mirror::Object* obj) {
   return object_logs_.GetOrCreate(obj, [&]() { return ObjectLog(&allocator_); });
 }
@@ -175,7 +211,7 @@ void Transaction::RecordWriteFieldBoolean(mirror::Object* obj,
                                           bool is_volatile) {
   DCHECK(obj != nullptr);
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(obj)) {
+  if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogBooleanValue(field_offset, value, is_volatile);
   }
@@ -187,7 +223,7 @@ void Transaction::RecordWriteFieldByte(mirror::Object* obj,
                                        bool is_volatile) {
   DCHECK(obj != nullptr);
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(obj)) {
+  if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogByteValue(field_offset, value, is_volatile);
   }
@@ -199,7 +235,7 @@ void Transaction::RecordWriteFieldChar(mirror::Object* obj,
                                        bool is_volatile) {
   DCHECK(obj != nullptr);
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(obj)) {
+  if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogCharValue(field_offset, value, is_volatile);
   }
@@ -212,7 +248,7 @@ void Transaction::RecordWriteFieldShort(mirror::Object* obj,
                                         bool is_volatile) {
   DCHECK(obj != nullptr);
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(obj)) {
+  if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogShortValue(field_offset, value, is_volatile);
   }
@@ -225,7 +261,7 @@ void Transaction::RecordWriteField32(mirror::Object* obj,
                                      bool is_volatile) {
   DCHECK(obj != nullptr);
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(obj)) {
+  if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.Log32BitsValue(field_offset, value, is_volatile);
   }
@@ -237,7 +273,7 @@ void Transaction::RecordWriteField64(mirror::Object* obj,
                                      bool is_volatile) {
   DCHECK(obj != nullptr);
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(obj)) {
+  if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.Log64BitsValue(field_offset, value, is_volatile);
   }
@@ -249,7 +285,7 @@ void Transaction::RecordWriteFieldReference(mirror::Object* obj,
                                             bool is_volatile) {
   DCHECK(obj != nullptr);
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(obj)) {
+  if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogReferenceValue(field_offset, value, is_volatile);
   }
@@ -260,7 +296,7 @@ void Transaction::RecordWriteArray(mirror::Array* array, size_t index, uint64_t
   DCHECK(array->IsArrayInstance());
   DCHECK(!array->IsObjectArray());
   DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
-  if (NeedsTransactionRecords(array)) {
+  if (array != last_allocated_object_) {
     ArrayLog& array_log = array_logs_.GetOrCreate(array, [&]() { return ArrayLog(&allocator_); });
     array_log.LogValue(index, value);
   }
@@ -489,6 +525,9 @@ void Transaction::ObjectLog::LogValue(ObjectLog::FieldValueKind kind,
                                       MemberOffset offset,
                                       uint64_t value,
                                       bool is_volatile) {
+  if (is_new_object_) {
+    return;
+  }
   auto it = field_values_.find(offset.Uint32Value());
   if (it == field_values_.end()) {
     ObjectLog::FieldValue field_value;
@@ -697,6 +736,9 @@ Transaction::InternStringLog::InternStringLog(ObjPtr<mirror::String> s,
 }
 
 void Transaction::ArrayLog::LogValue(size_t index, uint64_t value) {
+  if (is_new_array_) {
+    return;
+  }
   // Add a mapping if there is none yet.
   array_values_.FindOrAdd(index, value);
 }
diff --git a/runtime/transaction.h b/runtime/transaction.h
index a6de63cf74..60d1b3e3fd 100644
--- a/runtime/transaction.h
+++ b/runtime/transaction.h
@@ -74,22 +74,23 @@ class Transaction final {
     return strict_;
   }
 
-  // Record allocated object.
+  // Record newly allocated object/array.
   //
   // There is no reason to record old values for newly allocated objects because they become
   // unreachable when the transaction is rolled back, so their data does not need to be rolled back.
-  // We keep information about the last allocated object to reduce the number of such unnecessary
-  // records but we do not keep track of all objects newly allocated in the transaction.
-  // Note that just keeping the last allocated object avoids all records for a `System.arraycopy()`
-  // or `fill-array-data` used to copy data to a newly allocated array.
-  void RecordAllocatedObject(ObjPtr<mirror::Object> allocated_object)
-      REQUIRES_SHARED(Locks::mutator_lock_) {
-    last_allocated_object_ = allocated_object.Ptr();
-  }
+  //
+  // Implementation details: We track all newly allocated objects/arrays by creating an
+  // `ObjectLog`/`ArrayLog` flagged as a new object/array. We also cache the last allocated
+  // object/array which often helps avoid the search for the flagged `ObjectLog`/`ArrayLog`.
+  void RecordNewObject(ObjPtr<mirror::Object> allocated_object)
+      REQUIRES_SHARED(Locks::mutator_lock_);
+  void RecordNewArray(ObjPtr<mirror::Array> allocated_object)
+      REQUIRES_SHARED(Locks::mutator_lock_);
 
-  bool NeedsTransactionRecords(mirror::Object* obj) {
-    return obj != last_allocated_object_;
-  }
+  bool ObjectNeedsTransactionRecords(ObjPtr<mirror::Object> obj)
+      REQUIRES_SHARED(Locks::mutator_lock_);
+  bool ArrayNeedsTransactionRecords(ObjPtr<mirror::Array> array)
+      REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Record object field changes.
   void RecordWriteFieldBoolean(mirror::Object* obj,
@@ -177,8 +178,18 @@ class Transaction final {
       return field_values_.size();
     }
 
+    void MarkAsNewObject() {
+      DCHECK(field_values_.empty());
+      is_new_object_ = true;
+    }
+
+    bool IsNewObject() const {
+      return is_new_object_;
+    }
+
     explicit ObjectLog(ScopedArenaAllocator* allocator)
-        : field_values_(std::less<uint32_t>(), allocator->Adapter(kArenaAllocTransaction)) {}
+        : is_new_object_(false),
+          field_values_(std::less<uint32_t>(), allocator->Adapter(kArenaAllocTransaction)) {}
     ObjectLog(ObjectLog&& log) = default;
 
    private:
@@ -209,6 +220,10 @@ class Transaction final {
                         MemberOffset field_offset,
                         const FieldValue& field_value) const REQUIRES_SHARED(Locks::mutator_lock_);
 
+    // Whether this is a new object. We do not need to keep transaction records for objects
+    // created inside a transaction because they become unreachable on rollback.
+    bool is_new_object_;
+
     // Maps field's offset to its value.
     ScopedArenaSafeMap<uint32_t, FieldValue> field_values_;
 
@@ -225,8 +240,18 @@ class Transaction final {
       return array_values_.size();
     }
 
+    void MarkAsNewArray() {
+      DCHECK(array_values_.empty());
+      is_new_array_ = true;
+    }
+
+    bool IsNewArray() const {
+      return is_new_array_;
+    }
+
     explicit ArrayLog(ScopedArenaAllocator* allocator)
-        : array_values_(std::less<size_t>(), allocator->Adapter(kArenaAllocTransaction)) {}
+        : is_new_array_(false),
+          array_values_(std::less<size_t>(), allocator->Adapter(kArenaAllocTransaction)) {}
 
     ArrayLog(ArrayLog&& log) = default;
 
@@ -236,6 +261,10 @@ class Transaction final {
                         size_t index,
                         uint64_t value) const REQUIRES_SHARED(Locks::mutator_lock_);
 
+    // Whether this is a new array. We do not need to keep transaction records for arrays
+    // created inside a transaction because they become unreachable on rollback.
+    bool is_new_array_;
+
     // Maps index to value.
     // TODO use JValue instead ?
     ScopedArenaSafeMap<size_t, uint64_t> array_values_;