[vm] Ensure only generic closures have empty delayed type arguments.

Closures created via Closure::New() set the delayed type arguments to
the null type argument vector for non-generic closure functions and to
the empty type argument vector for generic closure functions.  However,
closures created via the method extractor stub would always have the
empty type argument vector for delayed type arguments.  Make the two
consistent by creating two method extractor stubs, one for generic
extracted methods and one for non-generic ones.

With this change, Closure::IsGeneric simply compares the delayed type
arguments against the empty type argument vector. This change also makes
method extraction for non-generic functions slightly faster, since the
delayed type arguments field is not changed post-allocation.

The old version of Closure::IsGeneric was the only use of
Closure::NumTypeParameters, so remove the latter method.

Related cleanups:

* Use similar checks for Class::IsGeneric and FunctionType::IsGeneric,
  since they only have non-null type_parameters fields when generic.
  For Class::IsGeneric, this avoids having to fetch the current thread
  if the class declaration has been loaded.

* Refactor methods that read packed fields in FunctionType into two
  methods, a static method that takes a FunctionTypePtr and an instance
  method that calls the static method. Replace the methods in Function
  that also directly read FunctionType packed fields with delegations to
  the static methods.

* Rework Closure_equals to avoid unneeded handle allocation in certain
  cases for non-equal closures.

* Replace cases that allocates a handle for the closure function only
  for retrieving the signature with calls to Closure::signature().

TEST=language_2/closure/tearoff_dynamic_test
     language_2/regress/regress45890_test

Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-x64c-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try,vm-kernel-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try
Change-Id: I29c8859c3350ed7b3f1a8f71d82a4393496b7c2b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206820
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>

runtime/lib/function.cc

@@ -30,42 +30,52 @@ DEFINE_NATIVE_ENTRY(Function_apply, 0, 2) {
   return result.ptr();
 }
 
+static bool ClosureEqualsHelper(Zone* zone,
+                                const Closure& receiver,
+                                const Object& other) {
+  if (receiver.ptr() == other.ptr()) {
+    return true;
+  }
+  if (!other.IsClosure()) {
+    return false;
+  }
+  const auto& other_closure = Closure::Cast(other);
+  // Check that the delayed type argument vectors match.
+  if (receiver.delayed_type_arguments() !=
+      other_closure.delayed_type_arguments()) {
+    // Mismatches should only happen when a generic function is involved.
+    ASSERT(Function::Handle(receiver.function()).IsGeneric() ||
+           Function::Handle(other_closure.function()).IsGeneric());
+    return false;
+  }
+  // Closures that are not implicit instance closures are unique.
+  const auto& func_a = Function::Handle(zone, receiver.function());
+  if (!func_a.IsImplicitInstanceClosureFunction()) {
+    return false;
+  }
+  const auto& func_b = Function::Handle(zone, other_closure.function());
+  if (!func_b.IsImplicitInstanceClosureFunction()) {
+    return false;
+  }
+  // If the closure functions are not the same, check the function's name and
+  // owner, as multiple function objects could exist for the same function due
+  // to hot reload.
+  if (func_a.ptr() != func_b.ptr() &&
+      (func_a.name() != func_b.name() || func_a.Owner() != func_b.Owner())) {
+    return false;
+  }
+  // Check that the both receiver instances are the same.
+  const Context& context_a = Context::Handle(zone, receiver.context());
+  const Context& context_b = Context::Handle(zone, other_closure.context());
+  return context_a.At(0) == context_b.At(0);
+}
+
 DEFINE_NATIVE_ENTRY(Closure_equals, 0, 2) {
   const Closure& receiver =
       Closure::CheckedHandle(zone, arguments->NativeArgAt(0));
   GET_NATIVE_ARGUMENT(Instance, other, arguments->NativeArgAt(1));
   ASSERT(!other.IsNull());
-  // For implicit instance closures compare receiver instance and function's
-  // name and owner (multiple function objects could exist for the same
-  // function due to hot reload).
-  // Objects of other closure kinds are unique, so use identity comparison.
-  if (receiver.ptr() == other.ptr()) {
-    return Bool::True().ptr();
-  }
-  if (other.IsClosure()) {
-    const Function& func_a = Function::Handle(zone, receiver.function());
-    if (func_a.IsImplicitInstanceClosureFunction()) {
-      const Closure& other_closure = Closure::Cast(other);
-      const Function& func_b = Function::Handle(zone, other_closure.function());
-      if (func_b.IsImplicitInstanceClosureFunction()) {
-        const Context& context_a = Context::Handle(zone, receiver.context());
-        const Context& context_b =
-            Context::Handle(zone, other_closure.context());
-        ObjectPtr receiver_a = context_a.At(0);
-        ObjectPtr receiver_b = context_b.At(0);
-        if ((receiver_a == receiver_b) &&
-            (!func_a.IsGeneric() ||
-             receiver.delayed_type_arguments() ==
-                 other_closure.delayed_type_arguments()) &&
-            ((func_a.ptr() == func_b.ptr()) ||
-             ((func_a.name() == func_b.name()) &&
-              (func_a.Owner() == func_b.Owner())))) {
-          return Bool::True().ptr();
-        }
-      }
-    }
-  }
-  return Bool::False().ptr();
+  return Bool::Get(ClosureEqualsHelper(zone, receiver, other)).ptr();
 }
 
 DEFINE_NATIVE_ENTRY(Closure_computeHash, 0, 1) {

runtime/lib/object.cc

@@ -114,17 +114,17 @@ DEFINE_NATIVE_ENTRY(Object_haveSameRuntimeType, 0, 2) {
 
   const Class& cls = Class::Handle(left.clazz());
   if (cls.IsClosureClass()) {
-    const Function& left_function =
-        Function::Handle(zone, Closure::Cast(left).function());
-    const Function& right_function =
-        Function::Handle(zone, Closure::Cast(right).function());
-    if (left_function.signature() == right_function.signature() &&
-        Closure::Cast(left).function_type_arguments() ==
-            Closure::Cast(right).function_type_arguments() &&
-        Closure::Cast(left).delayed_type_arguments() ==
-            Closure::Cast(right).delayed_type_arguments() &&
-        Closure::Cast(left).instantiator_type_arguments() ==
-            Closure::Cast(right).instantiator_type_arguments()) {
+    const auto& left_closure = Closure::Cast(left);
+    const auto& right_closure = Closure::Cast(right);
+    // If all the components that make up the instantiated signature are equal,
+    // then no need to instantiate.
+    if (left_closure.signature() == right_closure.signature() &&
+        left_closure.function_type_arguments() ==
+            right_closure.function_type_arguments() &&
+        left_closure.delayed_type_arguments() ==
+            right_closure.delayed_type_arguments() &&
+        left_closure.instantiator_type_arguments() ==
+            right_closure.instantiator_type_arguments()) {
       return Bool::True().ptr();
     }
     const AbstractType& left_type =
@@ -462,6 +462,7 @@ DEFINE_NATIVE_ENTRY(Internal_boundsCheckForPartialInstantiation, 0, 2) {
   const Closure& closure =
       Closure::CheckedHandle(zone, arguments->NativeArgAt(0));
   const Function& target = Function::Handle(zone, closure.function());
+  ASSERT(target.IsGeneric());  // No need to check bounds for non-generics.
   const TypeParameters& type_params =
       TypeParameters::Handle(zone, target.type_parameters());
   if (type_params.IsNull() || type_params.AllDynamicBounds()) {

runtime/vm/class_finalizer.cc

@@ -1761,7 +1761,8 @@ void ClassFinalizer::ClearAllCode(bool including_nonchanging_cids) {
   if (including_nonchanging_cids) {
     auto object_store = isolate_group->object_store();
     auto& null_code = Code::Handle(zone);
-    object_store->set_build_method_extractor_code(null_code);
+    object_store->set_build_generic_method_extractor_code(null_code);
+    object_store->set_build_nongeneric_method_extractor_code(null_code);
   }
 }
 

runtime/vm/compiler/aot/precompiler.cc

@@ -522,10 +522,18 @@ void Precompiler::DoCompileAll() {
 
         {
           SafepointWriteRwLocker ml(T, T->isolate_group()->program_lock());
-          stub_code = StubCode::GetBuildMethodExtractorStub(
+          stub_code = StubCode::GetBuildGenericMethodExtractorStub(
               global_object_pool_builder());
         }
-        IG->object_store()->set_build_method_extractor_code(stub_code);
+        IG->object_store()->set_build_generic_method_extractor_code(stub_code);
+
+        {
+          SafepointWriteRwLocker ml(T, T->isolate_group()->program_lock());
+          stub_code = StubCode::GetBuildNonGenericMethodExtractorStub(
+              global_object_pool_builder());
+        }
+        IG->object_store()->set_build_nongeneric_method_extractor_code(
+            stub_code);
       }
 
       CollectDynamicFunctionNames();

runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

@@ -252,8 +252,14 @@ void FlowGraphCompiler::GenerateMethodExtractorIntrinsic(
   ASSERT(!__ constant_pool_allowed());
   ASSERT(extracted_method.IsZoneHandle());
 
-  const Code& build_method_extractor = Code::ZoneHandle(
-      isolate_group()->object_store()->build_method_extractor_code());
+  const Code& build_method_extractor =
+      Code::ZoneHandle(extracted_method.IsGeneric()
+                           ? isolate_group()
+                                 ->object_store()
+                                 ->build_generic_method_extractor_code()
+                           : isolate_group()
+                                 ->object_store()
+                                 ->build_nongeneric_method_extractor_code());
 
   const intptr_t stub_index = __ object_pool_builder().AddObject(
       build_method_extractor, ObjectPool::Patchability::kNotPatchable);

runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc

@@ -243,8 +243,14 @@ void FlowGraphCompiler::GenerateMethodExtractorIntrinsic(
   ASSERT(!__ constant_pool_allowed());
   ASSERT(extracted_method.IsZoneHandle());
 
-  const Code& build_method_extractor = Code::ZoneHandle(
-      isolate_group()->object_store()->build_method_extractor_code());
+  const Code& build_method_extractor =
+      Code::ZoneHandle(extracted_method.IsGeneric()
+                           ? isolate_group()
+                                 ->object_store()
+                                 ->build_generic_method_extractor_code()
+                           : isolate_group()
+                                 ->object_store()
+                                 ->build_nongeneric_method_extractor_code());
 
   const intptr_t stub_index = __ object_pool_builder().AddObject(
       build_method_extractor, ObjectPool::Patchability::kNotPatchable);

runtime/vm/compiler/backend/flow_graph_compiler_x64.cc

@@ -252,8 +252,14 @@ void FlowGraphCompiler::GenerateMethodExtractorIntrinsic(
   ASSERT(!__ constant_pool_allowed());
   ASSERT(extracted_method.IsZoneHandle());
 
-  const Code& build_method_extractor = Code::ZoneHandle(
-      isolate_group()->object_store()->build_method_extractor_code());
+  const Code& build_method_extractor =
+      Code::ZoneHandle(extracted_method.IsGeneric()
+                           ? isolate_group()
+                                 ->object_store()
+                                 ->build_generic_method_extractor_code()
+                           : isolate_group()
+                                 ->object_store()
+                                 ->build_nongeneric_method_extractor_code());
   ASSERT(!build_method_extractor.IsNull());
 
   const intptr_t stub_index = __ object_pool_builder().AddObject(

runtime/vm/compiler/frontend/kernel_translation_helper.cc

@@ -3389,6 +3389,10 @@ void TypeTranslator::LoadAndSetupTypeParameters(
   ASSERT(parameterized_class.IsNull() != parameterized_signature.IsNull());
   ASSERT(type_parameter_count >= 0);
   if (type_parameter_count == 0) {
+    ASSERT(parameterized_class.IsNull() ||
+           parameterized_class.type_parameters() == TypeParameters::null());
+    ASSERT(parameterized_signature.IsNull() ||
+           parameterized_signature.type_parameters() == TypeParameters::null());
     return;
   }
 

runtime/vm/compiler/stub_code_compiler.h

@@ -54,7 +54,8 @@ class StubCodeCompiler : public AllStatic {
   static void GenerateBuildMethodExtractorStub(
       Assembler* assembler,
       const Code& closure_allocation_stub,
-      const Code& context_allocation_stub);
+      const Code& context_allocation_stub,
+      bool generic);
 #endif
 
   static void EnsureIsNewOrRemembered(Assembler* assembler,

runtime/vm/compiler/stub_code_compiler_arm.cc

@@ -224,7 +224,8 @@ void StubCodeCompiler::GenerateSharedStub(
 void StubCodeCompiler::GenerateBuildMethodExtractorStub(
     Assembler* assembler,
     const Code& closure_allocation_stub,
-    const Code& context_allocation_stub) {
+    const Code& context_allocation_stub,
+    bool generic) {
   const intptr_t kReceiverOffset = target::frame_layout.param_end_from_fp + 1;
 
   __ EnterStubFrame();
@@ -291,12 +292,15 @@ void StubCodeCompiler::GenerateBuildMethodExtractorStub(
       FieldAddress(AllocateClosureABI::kResultReg,
                    target::Closure::instantiator_type_arguments_offset()),
       AllocateClosureABI::kScratchReg);
-  __ LoadObject(AllocateClosureABI::kScratchReg, EmptyTypeArguments());
-  __ StoreIntoObjectNoBarrier(
-      AllocateClosureABI::kResultReg,
-      FieldAddress(AllocateClosureABI::kResultReg,
-                   target::Closure::delayed_type_arguments_offset()),
-      AllocateClosureABI::kScratchReg);
+  // Keep delayed_type_arguments as null if non-generic (see Closure::New).
+  if (generic) {
+    __ LoadObject(AllocateClosureABI::kScratchReg, EmptyTypeArguments());
+    __ StoreIntoObjectNoBarrier(
+        AllocateClosureABI::kResultReg,
+        FieldAddress(AllocateClosureABI::kResultReg,
+                     target::Closure::delayed_type_arguments_offset()),
+        AllocateClosureABI::kScratchReg);
+  }
 
   __ LeaveStubFrame();
   // No-op if the two are the same.

runtime/vm/compiler/stub_code_compiler_arm64.cc

@@ -460,7 +460,8 @@ void StubCodeCompiler::GenerateJITCallbackTrampolines(
 void StubCodeCompiler::GenerateBuildMethodExtractorStub(
     Assembler* assembler,
     const Code& closure_allocation_stub,
-    const Code& context_allocation_stub) {
+    const Code& context_allocation_stub,
+    bool generic) {
   const intptr_t kReceiverOffset = target::frame_layout.param_end_from_fp + 1;
 
   __ EnterStubFrame();
@@ -530,12 +531,15 @@ void StubCodeCompiler::GenerateBuildMethodExtractorStub(
       FieldAddress(AllocateClosureABI::kResultReg,
                    target::Closure::instantiator_type_arguments_offset()),
       AllocateClosureABI::kScratchReg);
-  __ LoadObject(AllocateClosureABI::kScratchReg, EmptyTypeArguments());
-  __ StoreCompressedIntoObjectNoBarrier(
-      AllocateClosureABI::kResultReg,
-      FieldAddress(AllocateClosureABI::kResultReg,
-                   target::Closure::delayed_type_arguments_offset()),
-      AllocateClosureABI::kScratchReg);
+  // Keep delayed_type_arguments as null if non-generic (see Closure::New).
+  if (generic) {
+    __ LoadObject(AllocateClosureABI::kScratchReg, EmptyTypeArguments());
+    __ StoreCompressedIntoObjectNoBarrier(
+        AllocateClosureABI::kResultReg,
+        FieldAddress(AllocateClosureABI::kResultReg,
+                     target::Closure::delayed_type_arguments_offset()),
+        AllocateClosureABI::kScratchReg);
+  }
 
   __ LeaveStubFrame();
   // No-op if the two are the same.

runtime/vm/compiler/stub_code_compiler_x64.cc

@@ -403,7 +403,8 @@ void StubCodeCompiler::GenerateJITCallbackTrampolines(
 void StubCodeCompiler::GenerateBuildMethodExtractorStub(
     Assembler* assembler,
     const Code& closure_allocation_stub,
-    const Code& context_allocation_stub) {
+    const Code& context_allocation_stub,
+    bool generic) {
   const intptr_t kReceiverOffsetInWords =
       target::frame_layout.param_end_from_fp + 1;
 
@@ -471,12 +472,15 @@ void StubCodeCompiler::GenerateBuildMethodExtractorStub(
       FieldAddress(AllocateClosureABI::kResultReg,
                    target::Closure::instantiator_type_arguments_offset()),
       AllocateClosureABI::kScratchReg);
-  __ LoadObject(AllocateClosureABI::kScratchReg, EmptyTypeArguments());
-  __ StoreCompressedIntoObjectNoBarrier(
-      AllocateClosureABI::kResultReg,
-      FieldAddress(AllocateClosureABI::kResultReg,
-                   target::Closure::delayed_type_arguments_offset()),
-      AllocateClosureABI::kScratchReg);
+  // Keep delayed_type_arguments as null if non-generic (see Closure::New).
+  if (generic) {
+    __ LoadObject(AllocateClosureABI::kScratchReg, EmptyTypeArguments());
+    __ StoreCompressedIntoObjectNoBarrier(
+        AllocateClosureABI::kResultReg,
+        FieldAddress(AllocateClosureABI::kResultReg,
+                     target::Closure::delayed_type_arguments_offset()),
+        AllocateClosureABI::kScratchReg);
+  }
 
   __ LeaveStubFrame();
   // No-op if the two are the same.

runtime/vm/dart.cc

@@ -871,16 +871,32 @@ ErrorPtr Dart::InitializeIsolate(const uint8_t* snapshot_data,
 
   if (kIsAotRuntime || was_child_cloned_into_existing_isolate) {
 #if !defined(TARGET_ARCH_IA32)
-    ASSERT(IG->object_store()->build_method_extractor_code() != Code::null());
+    ASSERT(IG->object_store()->build_generic_method_extractor_code() !=
+           Code::null());
+    ASSERT(IG->object_store()->build_nongeneric_method_extractor_code() !=
+           Code::null());
 #endif
   } else {
 #if !defined(TARGET_ARCH_IA32)
     if (I != Dart::vm_isolate()) {
-      if (IG->object_store()->build_method_extractor_code() != nullptr) {
+      if (IG->object_store()->build_generic_method_extractor_code() !=
+          nullptr) {
         SafepointWriteRwLocker ml(T, IG->program_lock());
-        if (IG->object_store()->build_method_extractor_code() != nullptr) {
-          IG->object_store()->set_build_method_extractor_code(
-              Code::Handle(StubCode::GetBuildMethodExtractorStub(nullptr)));
+        if (IG->object_store()->build_generic_method_extractor_code() !=
+            nullptr) {
+          IG->object_store()->set_build_generic_method_extractor_code(
+              Code::Handle(
+                  StubCode::GetBuildGenericMethodExtractorStub(nullptr)));
+        }
+      }
+      if (IG->object_store()->build_nongeneric_method_extractor_code() !=
+          nullptr) {
+        SafepointWriteRwLocker ml(T, IG->program_lock());
+        if (IG->object_store()->build_nongeneric_method_extractor_code() !=
+            nullptr) {
+          IG->object_store()->set_build_nongeneric_method_extractor_code(
+              Code::Handle(
+                  StubCode::GetBuildNonGenericMethodExtractorStub(nullptr)));
         }
       }
     }

runtime/vm/dart_entry.cc

@@ -222,7 +222,7 @@ ObjectPtr DartEntry::ResolveCallable(Thread* thread,
       if (matches && type_args_len > 0 && function.IsClosureFunction()) {
         // Though the closure function is generic, the closure itself may
         // not be because it closes over delayed function type arguments.
-        matches = Closure::Cast(instance).IsGeneric(thread);
+        matches = Closure::Cast(instance).IsGeneric();
       }
 
       if (matches) {

runtime/vm/isolate_reload.cc

@@ -2222,13 +2222,11 @@ class FieldInvalidator {
     cls_ = value.clazz();
     const intptr_t cid = cls_.id();
     if (cid == kClosureCid) {
-      instance_cid_or_signature_ = Closure::Cast(value).signature();
-      instance_type_arguments_ =
-          Closure::Cast(value).instantiator_type_arguments();
-      parent_function_type_arguments_ =
-          Closure::Cast(value).function_type_arguments();
-      delayed_function_type_arguments_ =
-          Closure::Cast(value).delayed_type_arguments();
+      const auto& closure = Closure::Cast(value);
+      instance_cid_or_signature_ = closure.signature();
+      instance_type_arguments_ = closure.instantiator_type_arguments();
+      parent_function_type_arguments_ = closure.function_type_arguments();
+      delayed_function_type_arguments_ = closure.delayed_type_arguments();
     } else {
       instance_cid_or_signature_ = Smi::New(cid);
       if (cls_.NumTypeArguments() > 0) {

runtime/vm/object.cc

@@ -7961,19 +7961,11 @@ intptr_t FunctionType::GetRequiredFlagIndex(intptr_t index,
 bool Function::HasRequiredNamedParameters() const {
 #if defined(DART_PRECOMPILED_RUNTIME)
   if (signature() == FunctionType::null()) {
-    // Signature is not dropped in aot when any named parameter is required.
+    // Signatures for functions with required named parameters are not dropped.
     return false;
   }
 #endif
-  if (!HasOptionalNamedParameters()) {
-    return false;
-  }
-  const FunctionType& sig = FunctionType::Handle(signature());
-  const Array& parameter_names = Array::Handle(sig.named_parameter_names());
-  if (parameter_names.IsNull()) {
-    return false;
-  }
-  return parameter_names.Length() > NumOptionalNamedParameters();
+  return FunctionType::Handle(signature()).HasRequiredNamedParameters();
 }
 
 bool Function::IsRequiredAt(intptr_t index) const {
@@ -8043,6 +8035,15 @@ void FunctionType::FinalizeNameArray() const {
 #endif
 }
 
+bool FunctionType::HasRequiredNamedParameters() const {
+  const intptr_t num_named_params = NumOptionalNamedParameters();
+  if (num_named_params == 0) return false;
+  // Check for flag slots in the named parameter names array.
+  const auto& parameter_names = Array::Handle(named_parameter_names());
+  ASSERT(!parameter_names.IsNull());
+  return parameter_names.Length() > num_named_params;
+}
+
 static void ReportTooManyTypeParameters(const FunctionType& sig) {
   Report::MessageF(Report::kError, Script::Handle(), TokenPosition::kNoSource,
                    Report::AtLocation,
@@ -8070,10 +8071,41 @@ void FunctionType::SetNumParentTypeArguments(intptr_t value) const {
       value);
 }
 
+bool Function::IsGeneric() const {
+  return FunctionType::IsGeneric(signature());
+}
+intptr_t Function::NumTypeParameters() const {
+  return FunctionType::NumTypeParametersOf(signature());
+}
 intptr_t Function::NumParentTypeArguments() const {
-  // Don't allocate handle in cases where we know it is 0.
-  if (!IsClosureFunction()) return 0;
-  return FunctionType::Handle(signature()).NumParentTypeArguments();
+  return FunctionType::NumParentTypeArgumentsOf(signature());
+}
+intptr_t Function::NumTypeArguments() const {
+  return FunctionType::NumTypeArgumentsOf(signature());
+}
+intptr_t Function::num_fixed_parameters() const {
+  return FunctionType::NumFixedParametersOf(signature());
+}
+bool Function::HasOptionalParameters() const {
+  return FunctionType::HasOptionalParameters(signature());
+}
+bool Function::HasOptionalNamedParameters() const {
+  return FunctionType::HasOptionalNamedParameters(signature());
+}
+bool Function::HasOptionalPositionalParameters() const {
+  return FunctionType::HasOptionalPositionalParameters(signature());
+}
+intptr_t Function::NumOptionalParameters() const {
+  return FunctionType::NumOptionalParametersOf(signature());
+}
+intptr_t Function::NumOptionalPositionalParameters() const {
+  return FunctionType::NumOptionalPositionalParametersOf(signature());
+}
+intptr_t Function::NumOptionalNamedParameters() const {
+  return FunctionType::NumOptionalNamedParametersOf(signature());
+}
+intptr_t Function::NumParameters() const {
+  return FunctionType::NumParametersOf(signature());
 }
 
 TypeParameterPtr Function::TypeParameterAt(intptr_t index,
@@ -8167,10 +8199,6 @@ bool Function::CanBeInlined() const {
 }
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
-intptr_t Function::NumParameters() const {
-  return num_fixed_parameters() + NumOptionalParameters();
-}
-
 intptr_t Function::NumImplicitParameters() const {
   const UntaggedFunction::Kind k = kind();
   if (k == UntaggedFunction::kConstructor) {
@@ -10239,10 +10267,6 @@ void FunctionType::set_packed_type_parameter_counts(
   untag()->packed_type_parameter_counts_ = packed_type_parameter_counts;
 }
 
-intptr_t FunctionType::NumParameters() const {
-  return num_fixed_parameters() + NumOptionalParameters();
-}
-
 void FunctionType::set_num_implicit_parameters(intptr_t value) const {
   ASSERT(value >= 0);
   untag()->packed_parameter_counts_.Update<PackedNumImplicitParameters>(value);
@@ -10297,6 +10321,8 @@ void FfiTrampolineData::set_callback_target(const Function& value) const {
 void FunctionType::SetNumOptionalParameters(
     intptr_t value,
     bool are_optional_positional) const {
+  // HasOptionalNamedParameters only checks this bit, so only set it if there
+  // are actual named parameters.
   untag()->packed_parameter_counts_.Update<PackedHasNamedOptionalParameters>(
       (value > 0) && !are_optional_positional);
   untag()->packed_parameter_counts_.Update<PackedNumOptionalParameters>(value);
@@ -25082,17 +25108,6 @@ void Closure::CanonicalizeFieldsLocked(Thread* thread) const {
   // Ignore function, context, hash.
 }
 
-intptr_t Closure::NumTypeParameters(Thread* thread) const {
-  // Only check for empty here, as the null TAV is used to mean that the
-  // closed-over delayed type parameters were all of dynamic type.
-  if (delayed_type_arguments() != Object::empty_type_arguments().ptr()) {
-    return 0;
-  } else {
-    const auto& closure_function = Function::Handle(thread->zone(), function());
-    return closure_function.NumTypeParameters();
-  }
-}
-
 const char* Closure::ToCString() const {
   auto const thread = Thread::Current();
   auto const zone = thread->zone();
@@ -25153,6 +25168,10 @@ ClosurePtr Closure::New(const TypeArguments& instantiator_type_arguments,
                         const Function& function,
                         const Context& context,
                         Heap::Space space) {
+  // We store null delayed type arguments, not empty ones, in closures with
+  // non-generic functions a) to make method extraction slightly faster and
+  // b) to make the Closure::IsGeneric check fast.
+  // Keep in sync with StubCodeCompiler::GenerateBuildMethodExtractorStub.
   return Closure::New(instantiator_type_arguments, function_type_arguments,
                       function.IsGeneric() ? Object::empty_type_arguments()
                                            : Object::null_type_arguments(),
@@ -25210,7 +25229,7 @@ FunctionTypePtr Closure::GetInstantiatedSignature(Zone* zone) const {
   // We detect the case of a partial tearoff type application and substitute the
   // type arguments for the type parameters of the function.
   intptr_t num_free_params;
-  if (delayed_type_args.ptr() != Object::empty_type_arguments().ptr()) {
+  if (!IsGeneric() && fun.IsGeneric()) {
     num_free_params = kCurrentAndEnclosingFree;
     fn_type_args = delayed_type_args.Prepend(
         zone, fn_type_args, sig.NumParentTypeArguments(),

runtime/vm/object.h

@@ -1175,7 +1175,13 @@ class Class : public Object {
   intptr_t NumTypeArguments() const;
 
   // Return true if this class declares type parameters.
-  bool IsGeneric() const { return NumTypeParameters(Thread::Current()) > 0; }
+  bool IsGeneric() const {
+    // If the declaration is not loaded, fall back onto NumTypeParameters.
+    if (!is_declaration_loaded()) {
+      return NumTypeParameters(Thread::Current()) > 0;
+    }
+    return type_parameters() != Object::null();
+  }
 
   // Returns a canonicalized vector of the type parameters instantiated
   // to bounds. If non-generic, the empty type arguments vector is returned.
@@ -2675,33 +2681,23 @@ class Function : public Object {
     return signature()->untag()->type_parameters();
   }
 
-  intptr_t NumTypeParameters() const {
-    return signature()
-        ->untag()
-        ->packed_type_parameter_counts_
-        .Read<UntaggedFunctionType::PackedNumTypeParameters>();
-  }
-
+  // Returns the number of local type arguments for this function.
+  intptr_t NumTypeParameters() const;
   // Return the cumulative number of type arguments in all parent functions.
   intptr_t NumParentTypeArguments() const;
-
-  // Return the cumulative number of type arguments in all parent functions and
-  // own type arguments.
-  intptr_t NumTypeArguments() const {
-    return NumParentTypeArguments() + NumTypeParameters();
-  }
+  // Return the cumulative number of type arguments for this function, including
+  // type arguments for all parent functions.
+  intptr_t NumTypeArguments() const;
+  // Return whether this function declares local type arguments.
+  bool IsGeneric() const;
+  // Returns whether any parent function of this function is generic.
+  bool HasGenericParent() const { return NumParentTypeArguments() > 0; }
 
   // Return the type parameter declared at index.
   TypeParameterPtr TypeParameterAt(
       intptr_t index,
       Nullability nullability = Nullability::kNonNullable) const;
 
-  // Return true if this function declares type parameters.
-  // Generic dispatchers only set the number without actual type parameters.
-  bool IsGeneric() const { return NumTypeParameters() > 0; }
-  // Return true if any parent function of this function is generic.
-  bool HasGenericParent() const { return NumParentTypeArguments() > 0; }
-
   // Not thread-safe; must be called in the main thread.
   // Sets function's code and code's function.
   void InstallOptimizedCode(const Code& code) const;
@@ -3001,46 +2997,25 @@ class Function : public Object {
     return OFFSET_OF(UntaggedFunction, packed_fields_);
   }
 
-  intptr_t num_fixed_parameters() const {
-    return signature()
-        ->untag()
-        ->packed_parameter_counts_
-        .Read<UntaggedFunctionType::PackedNumFixedParameters>();
-  }
-
-  bool HasOptionalParameters() const {
-    return signature()
-               ->untag()
-               ->packed_parameter_counts_
-               .Read<UntaggedFunctionType::PackedNumOptionalParameters>() > 0;
-  }
-  bool HasOptionalNamedParameters() const {
-    return HasOptionalParameters() &&
-           signature()
-               ->untag()
-               ->packed_parameter_counts_
-               .Read<UntaggedFunctionType::PackedHasNamedOptionalParameters>();
-  }
+  // Returns the number of required positional parameters.
+  intptr_t num_fixed_parameters() const;
+  // Returns the number of optional parameters, whether positional or named.
+  bool HasOptionalParameters() const;
+  // Returns whether the function has optional named parameters.
+  bool HasOptionalNamedParameters() const;
+  // Returns whether the fuction has required named parameters.
   bool HasRequiredNamedParameters() const;
-  bool HasOptionalPositionalParameters() const {
-    return HasOptionalParameters() && !HasOptionalNamedParameters();
-  }
-  intptr_t NumOptionalParameters() const {
-    return signature()
-        ->untag()
-        ->packed_parameter_counts_
-        .Read<UntaggedFunctionType::PackedNumOptionalParameters>();
-  }
-  intptr_t NumOptionalPositionalParameters() const {
-    return HasOptionalPositionalParameters() ? NumOptionalParameters() : 0;
-  }
-
-  intptr_t NumOptionalNamedParameters() const {
-    return HasOptionalNamedParameters() ? NumOptionalParameters() : 0;
-  }
-
+  // Returns whether the function has optional positional parameters.
+  bool HasOptionalPositionalParameters() const;
+  // Returns the number of optional parameters, or 0 if none.
+  intptr_t NumOptionalParameters() const;
+  // Returns the number of optional positional parameters, or 0 if none.
+  intptr_t NumOptionalPositionalParameters() const;
+  // Returns the number of optional named parameters, or 0 if none.
+  intptr_t NumOptionalNamedParameters() const;
+  // Returns the total number of both required and optional parameters.
   intptr_t NumParameters() const;
-
+  // Returns the number of implicit parameters, e.g., this for instance methods.
   intptr_t NumImplicitParameters() const;
 
 #if defined(DART_PRECOMPILED_RUNTIME)
@@ -8446,61 +8421,90 @@ class FunctionType : public AbstractType {
 
   bool IsSubtypeOf(const FunctionType& other, Heap::Space space) const;
 
-  intptr_t NumParameters() const;
-
-  // Return the number of type arguments in enclosing signature.
-  intptr_t NumParentTypeArguments() const {
-    return untag()
+  static intptr_t NumParentTypeArgumentsOf(FunctionTypePtr ptr) {
+    return ptr->untag()
         ->packed_type_parameter_counts_.Read<PackedNumParentTypeArguments>();
   }
+  // Return the number of type arguments in the enclosing signature.
+  intptr_t NumParentTypeArguments() const {
+    return NumParentTypeArgumentsOf(ptr());
+  }
   void SetNumParentTypeArguments(intptr_t value) const;
-  intptr_t NumTypeParameters() const {
-    return PackedNumTypeParameters::decode(
-        untag()->packed_type_parameter_counts_);
+  static intptr_t NumTypeParametersOf(FunctionTypePtr ptr) {
+    return ptr->untag()
+        ->packed_type_parameter_counts_.Read<PackedNumTypeParameters>();
   }
+  intptr_t NumTypeParameters() const { return NumTypeParametersOf(ptr()); }
 
-  intptr_t NumTypeArguments() const {
-    return NumParentTypeArguments() + NumTypeParameters();
+  static intptr_t NumTypeArgumentsOf(FunctionTypePtr ptr) {
+    return NumTypeParametersOf(ptr) + NumParentTypeArgumentsOf(ptr);
   }
+  intptr_t NumTypeArguments() const { return NumTypeArgumentsOf(ptr()); }
 
   intptr_t num_implicit_parameters() const {
     return untag()
         ->packed_parameter_counts_.Read<PackedNumImplicitParameters>();
   }
   void set_num_implicit_parameters(intptr_t value) const;
-  intptr_t num_fixed_parameters() const {
-    return untag()->packed_parameter_counts_.Read<PackedNumFixedParameters>();
+
+  static intptr_t NumFixedParametersOf(FunctionTypePtr ptr) {
+    return ptr->untag()
+        ->packed_parameter_counts_.Read<PackedNumFixedParameters>();
   }
+  intptr_t num_fixed_parameters() const { return NumFixedParametersOf(ptr()); }
   void set_num_fixed_parameters(intptr_t value) const;
 
-  bool HasOptionalParameters() const {
-    return untag()
+  static bool HasOptionalParameters(FunctionTypePtr ptr) {
+    return ptr->untag()
                ->packed_parameter_counts_.Read<PackedNumOptionalParameters>() >
            0;
   }
+  bool HasOptionalParameters() const { return HasOptionalParameters(ptr()); }
+
+  static bool HasOptionalNamedParameters(FunctionTypePtr ptr) {
+    return ptr->untag()
+        ->packed_parameter_counts_.Read<PackedHasNamedOptionalParameters>();
+  }
   bool HasOptionalNamedParameters() const {
-    return HasOptionalParameters() &&
-           untag()
-               ->packed_parameter_counts_
-               .Read<PackedHasNamedOptionalParameters>();
+    return HasOptionalNamedParameters(ptr());
+  }
+  bool HasRequiredNamedParameters() const;
+
+  static bool HasOptionalPositionalParameters(FunctionTypePtr ptr) {
+    return !HasOptionalNamedParameters(ptr) && HasOptionalParameters(ptr);
   }
   bool HasOptionalPositionalParameters() const {
-    return HasOptionalParameters() && !HasOptionalNamedParameters();
+    return HasOptionalPositionalParameters(ptr());
+  }
+
+  static intptr_t NumOptionalParametersOf(FunctionTypePtr ptr) {
+    return ptr->untag()
+        ->packed_parameter_counts_.Read<PackedNumOptionalParameters>();
   }
   intptr_t NumOptionalParameters() const {
-    return untag()
-        ->packed_parameter_counts_.Read<PackedNumOptionalParameters>();
+    return NumOptionalParametersOf(ptr());
   }
   void SetNumOptionalParameters(intptr_t num_optional_parameters,
                                 bool are_optional_positional) const;
 
+  static intptr_t NumOptionalPositionalParametersOf(FunctionTypePtr ptr) {
+    return HasOptionalNamedParameters(ptr) ? 0 : NumOptionalParametersOf(ptr);
+  }
   intptr_t NumOptionalPositionalParameters() const {
-    return HasOptionalPositionalParameters() ? NumOptionalParameters() : 0;
+    return NumOptionalPositionalParametersOf(ptr());
   }
 
-  intptr_t NumOptionalNamedParameters() const {
-    return HasOptionalNamedParameters() ? NumOptionalParameters() : 0;
+  static intptr_t NumOptionalNamedParametersOf(FunctionTypePtr ptr) {
+    return HasOptionalNamedParameters(ptr) ? NumOptionalParametersOf(ptr) : 0;
   }
+  intptr_t NumOptionalNamedParameters() const {
+    return NumOptionalNamedParametersOf(ptr());
+  }
+
+  static intptr_t NumParametersOf(FunctionTypePtr ptr) {
+    return NumFixedParametersOf(ptr) + NumOptionalParametersOf(ptr);
+  }
+  intptr_t NumParameters() const { return NumParametersOf(ptr()); }
 
   uint32_t packed_parameter_counts() const {
     return untag()->packed_parameter_counts_;
@@ -8595,7 +8599,10 @@ class FunctionType : public AbstractType {
                                       TrailPtr trail = nullptr) const;
 
   // Return true if this function type declares type parameters.
-  bool IsGeneric() const { return NumTypeParameters() > 0; }
+  static bool IsGeneric(FunctionTypePtr ptr) {
+    return ptr->untag()->type_parameters() != TypeParameters::null();
+  }
+  bool IsGeneric() const { return IsGeneric(ptr()); }
 
   // Return true if any enclosing signature of this signature is generic.
   bool HasGenericParent() const { return NumParentTypeArguments() > 0; }
@@ -11254,10 +11261,11 @@ class Closure : public Instance {
     return closure.untag()->context();
   }
 
-  bool IsGeneric(Thread* thread) const { return NumTypeParameters(thread) > 0; }
-  intptr_t NumTypeParameters(Thread* thread) const;
-  // No need for num_parent_type_arguments, as a closure is always closed
-  // over its parents type parameters (i.e., function_type_parameters() above).
+  // Returns whether the closure is generic, that is, it has a generic closure
+  // function and no delayed type arguments.
+  bool IsGeneric() const {
+    return delayed_type_arguments() == Object::empty_type_arguments().ptr();
+  }
 
   SmiPtr hash() const { return untag()->hash(); }
   static intptr_t hash_offset() { return OFFSET_OF(UntaggedClosure, hash_); }

runtime/vm/object_store.h

@@ -176,7 +176,8 @@ class ObjectPointerVisitor;
   RW(ObjectPool, global_object_pool)                                           \
   RW(Array, unique_dynamic_targets)                                            \
   RW(GrowableObjectArray, megamorphic_cache_table)                             \
-  RW(Code, build_method_extractor_code)                                        \
+  RW(Code, build_generic_method_extractor_code)                                \
+  RW(Code, build_nongeneric_method_extractor_code)                             \
   RW(Code, dispatch_table_null_error_stub)                                     \
   RW(Code, late_initialization_error_stub_with_fpu_regs_stub)                  \
   RW(Code, late_initialization_error_stub_without_fpu_regs_stub)               \

runtime/vm/runtime_entry.cc

@@ -758,8 +758,7 @@ static void UpdateTypeTestCache(
   auto& instance_delayed_type_arguments = TypeArguments::Handle(zone);
   if (instance_class.IsClosureClass()) {
     const auto& closure = Closure::Cast(instance);
-    const auto& closure_function = Function::Handle(zone, closure.function());
-    instance_class_id_or_signature = closure_function.signature();
+    instance_class_id_or_signature = closure.signature();
     instance_type_arguments = closure.instantiator_type_arguments();
     instance_parent_function_type_arguments = closure.function_type_arguments();
     instance_delayed_type_arguments = closure.delayed_type_arguments();

runtime/vm/stub_code.cc

@@ -299,8 +299,8 @@ CodePtr StubCode::GetAllocationStubForTypedData(classid_t class_id) {
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
 #if !defined(TARGET_ARCH_IA32)
-CodePtr StubCode::GetBuildMethodExtractorStub(
-    compiler::ObjectPoolBuilder* pool) {
+CodePtr StubCode::GetBuildMethodExtractorStub(compiler::ObjectPoolBuilder* pool,
+                                              bool generic) {
 #if !defined(DART_PRECOMPILED_RUNTIME)
   auto thread = Thread::Current();
   auto Z = thread->zone();
@@ -314,9 +314,10 @@ CodePtr StubCode::GetBuildMethodExtractorStub(
   compiler::ObjectPoolBuilder object_pool_builder;
   compiler::Assembler assembler(pool != nullptr ? pool : &object_pool_builder);
   compiler::StubCodeCompiler::GenerateBuildMethodExtractorStub(
-      &assembler, closure_allocation_stub, context_allocation_stub);
+      &assembler, closure_allocation_stub, context_allocation_stub, generic);
 
-  const char* name = "BuildMethodExtractor";
+  const char* name = generic ? "BuildGenericMethodExtractor"
+                             : "BuildNonGenericMethodExtractor";
   const Code& stub = Code::Handle(Code::FinalizeCodeAndNotify(
       name, nullptr, &assembler, Code::PoolAttachment::kNotAttachPool,
       /*optimized=*/false));
@@ -368,7 +369,8 @@ const char* StubCode::NameOfStub(uword entry_point) {
     return "_iso_stub_" #name "Stub";                                          \
   }
   OBJECT_STORE_STUB_CODE_LIST(MATCH)
-  MATCH(build_method_extractor_code, BuildMethodExtractor)
+  MATCH(build_generic_method_extractor_code, BuildGenericMethodExtractor)
+  MATCH(build_nongeneric_method_extractor_code, BuildNonGenericMethodExtractor)
 #undef MATCH
   return nullptr;
 }

runtime/vm/stub_code.h

@@ -72,7 +72,14 @@ class StubCode : public AllStatic {
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
 #if !defined(TARGET_ARCH_IA32)
-  static CodePtr GetBuildMethodExtractorStub(compiler::ObjectPoolBuilder* pool);
+  static CodePtr GetBuildGenericMethodExtractorStub(
+      compiler::ObjectPoolBuilder* pool) {
+    return GetBuildMethodExtractorStub(pool, /*generic=*/true);
+  }
+  static CodePtr GetBuildNonGenericMethodExtractorStub(
+      compiler::ObjectPoolBuilder* pool) {
+    return GetBuildMethodExtractorStub(pool, /*generic=*/false);
+  }
 #endif
 
 #if !defined(DART_PRECOMPILED_RUNTIME)
@@ -110,6 +117,9 @@ class StubCode : public AllStatic {
  private:
   friend class MegamorphicCacheTable;
 
+  static CodePtr GetBuildMethodExtractorStub(compiler::ObjectPoolBuilder* pool,
+                                             bool generic);
+
   enum {
 #define STUB_CODE_ENTRY(name) k##name##Index,
     VM_STUB_CODE_LIST(STUB_CODE_ENTRY)