commit 86d1261dff279acf84bc699c6c116189955b0076
author Tess Strickland <sstrickl@google.com> Wed Jul 12 07:37:16 2023 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jul 12 07:37:16 2023 +0000
tree da7a9068331297a66a2f0cf581c5859c39de92b6
parent 23f414520647df3719da1a94f19237955a0f5fc2

[vm] Clean up New() methods of Dart classes.

Goals of this CL, a followup to 2f63acea22:

* Ensure that X::New() depends on the initialization guarantees of
  Object::Allocate<X>(...):

  * Ptr fields are guaranteed to be initialized to Object::null().
  * Non-Ptr fields are guaranteed to be zero-initialized.

  In cases where the only uses of the allocated object before return
  was to perform unnecessary field assignments, X::New() just simply
  returns the result of Object::Allocate<X>(...).

  Otherwise, the old now-unnecessary assignments have been changed
  into ASSERTs so that they will be checked in DEBUG mode.

* Ensure that NoSafepointScopes are entered in X::New() only when
  necessary (e.g., to ensure fields used to calculate to(...)
  are properly set before being seen by pointer visitors as the GC
  may run when outside a NoSafepointScope).

  In particular, the often occurring pattern:

  auto& result = X::Handle();
  {
    auto raw = Object::Allocate<X>(...);
    NoSafepointScope no_safepoint;
    result = raw;
  }
  ...

  has been replaced with:

  const auto& result = X::Handle(Object::Allocate<X>(...));

* If a handle was allocated, the only uses of that handle before
  returning must be performed under a NoSafepointScope, and the same
  operations can be done directly on the object pointer, then do so
  and remove the unnecessary handle allocation.

Notable changes outside the above:

* Swapped ObjectPool::EntryType::{kImmediate,kTaggedObject} so that
  kImmediate has value 0, since Object::Allocate<ObjectPool>(len)
  zero-initializes the payload and without this change,
  ObjectPool::New() must set the entry types manually.

* Removed the old static ArrayPtr cached_array_ field on
  SubtypeTestCache as well as SubtypeTestCache::{Init,Cleanup} and
  instead added Object::empty_subtype_test_cache_array().

* Removed the no-arg Closure::New() method, which is never used.

* Inlined the no-arg Script::New() method into its only caller,
  one of the other Script::New() overloads.

TEST=ci

Issue: https://github.com/dart-lang/sdk/issues/52876
Change-Id: I079b4c9f73c7d2c0146c30cf2cd570b91a1ecf36
Cq-Include-Trybots: luci.dart.try:vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-linux-release-x64-try,vm-aot-linux-product-x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-ffi-qemu-linux-release-riscv64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313120
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>