diff --git a/pkg/dart2bytecode/docs/bytecode.md b/pkg/dart2bytecode/docs/bytecode.md index ed201ee0559..ca23d4c18d4 100644 --- a/pkg/dart2bytecode/docs/bytecode.md +++ b/pkg/dart2bytecode/docs/bytecode.md @@ -681,7 +681,10 @@ type ClosureDeclaration { type ClosureCode { UInt flags = (hasExceptionsTable, hasSourcePositions, hasLocalVariables, - capturesOnlyFinalNotLateVars) + capturesOnlyFinalNotLateVars, hasLocalFunctionId) + + if hasLocalFunctionId + UInt localFunctionId; UInt bytecodeSizeInBytes; Byte[bytecodeSizeInBytes] bytecodes; diff --git a/pkg/dart2bytecode/lib/bytecode_generator.dart b/pkg/dart2bytecode/lib/bytecode_generator.dart index b498f50e48f..fb9d9e8eb98 100644 --- a/pkg/dart2bytecode/lib/bytecode_generator.dart +++ b/pkg/dart2bytecode/lib/bytecode_generator.dart @@ -2546,12 +2546,14 @@ class BytecodeGenerator extends RecursiveVisitor { locals.leaveScope(); + assert(node.id != LocalFunctionId.invalid); closure.code = new ClosureCode( asm.bytecode, asm.exceptionsTable, finalizeSourcePositions(), finalizeLocalVariables(), - capturesOnlyFinalNotLateVars); + capturesOnlyFinalNotLateVars, + node.id.toInt()); _popAssemblerState(); diff --git a/pkg/dart2bytecode/lib/declarations.dart b/pkg/dart2bytecode/lib/declarations.dart index 99d9fdcaf0d..3dcf32b312e 100644 --- a/pkg/dart2bytecode/lib/declarations.dart +++ b/pkg/dart2bytecode/lib/declarations.dart @@ -1167,28 +1167,40 @@ class ClosureCode { static const hasSourcePositionsFlag = 1 << 1; static const hasLocalVariablesFlag = 1 << 2; static const capturesOnlyFinalNotLateVarsFlag = 1 << 3; + static const hasLocalFunctionIdFlag = 1 << 4; final Uint8List bytecodes; final ExceptionsTable exceptionsTable; final SourcePositions? sourcePositions; final LocalVariableTable? localVariables; final bool capturesOnlyFinalNotLateVars; + final int localFunctionId; bool get hasExceptionsTable => exceptionsTable.blocks.isNotEmpty; bool get hasSourcePositions => sourcePositions?.isNotEmpty ?? false; bool get hasLocalVariables => localVariables?.isNotEmpty ?? false; + bool get hasLocalFunctionId => localFunctionId > 0; int get flags => (hasExceptionsTable ? hasExceptionsTableFlag : 0) | (hasSourcePositions ? hasSourcePositionsFlag : 0) | (hasLocalVariables ? hasLocalVariablesFlag : 0) | - (capturesOnlyFinalNotLateVars ? capturesOnlyFinalNotLateVarsFlag : 0); + (capturesOnlyFinalNotLateVars ? capturesOnlyFinalNotLateVarsFlag : 0) | + (hasLocalFunctionId ? hasLocalFunctionIdFlag : 0); - ClosureCode(this.bytecodes, this.exceptionsTable, this.sourcePositions, - this.localVariables, this.capturesOnlyFinalNotLateVars); + ClosureCode( + this.bytecodes, + this.exceptionsTable, + this.sourcePositions, + this.localVariables, + this.capturesOnlyFinalNotLateVars, + this.localFunctionId); void write(BufferedWriter writer) { writer.writePackedUInt30(flags); + if (hasLocalFunctionId) { + writer.writePackedUInt30(localFunctionId); + } _writeBytecodeInstructions(writer, bytecodes); if (hasExceptionsTable) { exceptionsTable.write(writer); @@ -1203,6 +1215,9 @@ class ClosureCode { factory ClosureCode.read(BufferedReader reader) { final int flags = reader.readPackedUInt30(); + final localFunctionId = ((flags & hasLocalFunctionIdFlag) != 0) + ? reader.readPackedUInt30() + : -1; final Uint8List bytecodes = _readBytecodeInstructions(reader); final exceptionsTable = ((flags & hasExceptionsTableFlag) != 0) ? new ExceptionsTable.read(reader) @@ -1216,9 +1231,8 @@ class ClosureCode { final capturesOnlyFinalNotLateVars = (flags & capturesOnlyFinalNotLateVarsFlag) != 0; - return new ClosureCode( - bytecodes, exceptionsTable, sourcePositions, localVariables, - capturesOnlyFinalNotLateVars); + return new ClosureCode(bytecodes, exceptionsTable, sourcePositions, + localVariables, capturesOnlyFinalNotLateVars, localFunctionId); } @override diff --git a/runtime/vm/bytecode_reader.cc b/runtime/vm/bytecode_reader.cc index fa7018ce121..51536862007 100644 --- a/runtime/vm/bytecode_reader.cc +++ b/runtime/vm/bytecode_reader.cc @@ -307,6 +307,12 @@ void BytecodeReaderHelper::ReadCode(const Function& function, const bool captures_only_final_not_late_vars = (flags & ClosureCode::kCapturesOnlyFinalNotLateVarsFlag) != 0; + intptr_t local_function_id = -1; + if ((flags & ClosureCode::kHasLocalFunctionIdFlag) != 0) { + local_function_id = reader_.ReadUInt(); + ASSERT(local_function_id > 0); + } + // Read closure bytecode and attach to closure function. closure_bytecode = ReadBytecode(pool); @@ -326,7 +332,8 @@ void BytecodeReaderHelper::ReadCode(const Function& function, } } - ClosureFunctionsCache::AddClosureFunctionLocked(closure); + ClosureFunctionsCache::AddClosureFunctionLocked(closure, + local_function_id); } } } diff --git a/runtime/vm/bytecode_reader.h b/runtime/vm/bytecode_reader.h index eab46ce0502..86587014ca9 100644 --- a/runtime/vm/bytecode_reader.h +++ b/runtime/vm/bytecode_reader.h @@ -309,6 +309,7 @@ class BytecodeReaderHelper : public ValueObject { static const int kHasSourcePositionsFlag = 1 << 1; static const int kHasLocalVariablesFlag = 1 << 2; static const int kCapturesOnlyFinalNotLateVarsFlag = 1 << 3; + static const int kHasLocalFunctionIdFlag = 1 << 4; }; // Parameter flags, must be in sync with ParameterFlags constants in diff --git a/runtime/vm/closure_functions_cache.cc b/runtime/vm/closure_functions_cache.cc index 0bafede2de0..d4b1e7004b8 100644 --- a/runtime/vm/closure_functions_cache.cc +++ b/runtime/vm/closure_functions_cache.cc @@ -25,16 +25,16 @@ using FunctionHashMap = UnorderedHashMap; FunctionPtr ClosureFunctionsCache::LookupClosureFunction( const Function& member_function, - intptr_t kernel_offset) { + intptr_t local_function_id) { ASSERT(!member_function.HasBytecode()); auto thread = Thread::Current(); SafepointReadRwLocker ml(thread, thread->isolate_group()->program_lock()); - return LookupClosureFunctionLocked(member_function, kernel_offset); + return LookupClosureFunctionLocked(member_function, local_function_id); } FunctionPtr ClosureFunctionsCache::LookupClosureFunctionLocked( const Function& member_function, - intptr_t kernel_offset) { + intptr_t local_function_id) { ASSERT(!member_function.HasBytecode()); auto thread = Thread::Current(); auto zone = thread->zone(); @@ -59,7 +59,7 @@ FunctionPtr ClosureFunctionsCache::LookupClosureFunctionLocked( auto& result = Function::Handle(zone); IntHashMap map2(zone, map_array.ptr()); - result ^= map2.GetOrNull(Smi::Handle(zone, Smi::New(kernel_offset))); + result ^= map2.GetOrNull(Smi::Handle(zone, Smi::New(local_function_id))); map2.Release(); return result.ptr(); @@ -67,6 +67,7 @@ FunctionPtr ClosureFunctionsCache::LookupClosureFunctionLocked( void ClosureFunctionsCache::AddClosureFunctionLocked( const Function& function, + intptr_t local_function_id, bool allow_implicit_closure_functions /* = false */) { ASSERT(!Compiler::IsBackgroundCompilation()); @@ -88,13 +89,12 @@ void ClosureFunctionsCache::AddClosureFunctionLocked( function.IsNonImplicitClosureFunction()); closures.Add(function, Heap::kOld); - if (allow_implicit_closure_functions || function.HasBytecode()) { + if (local_function_id < 0) { return; } const Function& member_function = Function::Handle(zone, function.GetOutermostFunction()); - ASSERT(function.kernel_offset() > 0); auto& map_array = Array::Handle(zone, object_store->closure_functions_table()); @@ -107,8 +107,7 @@ void ClosureFunctionsCache::AddClosureFunctionLocked( map_array = HashTables::New(4, Heap::kOld); } IntHashMap map2(zone, map_array.ptr()); - map2.UpdateOrInsert(Smi::Handle(zone, Smi::New(function.kernel_offset())), - function); + map2.UpdateOrInsert(Smi::Handle(zone, Smi::New(local_function_id)), function); map.UpdateOrInsert(member_function, map2.Release()); object_store->set_closure_functions_table(map.Release()); } diff --git a/runtime/vm/closure_functions_cache.h b/runtime/vm/closure_functions_cache.h index 9991f716d93..6e13ce2459d 100644 --- a/runtime/vm/closure_functions_cache.h +++ b/runtime/vm/closure_functions_cache.h @@ -24,7 +24,7 @@ class FunctionPtr; // closure functions. // // The cache is currently implemented as a 2-level -// Map>. +// Map>. // // The function is also added to the growable list in order to // satisfy the following requirements: @@ -33,11 +33,13 @@ class FunctionPtr; // class ClosureFunctionsCache : public AllStatic { public: + static constexpr int kInvalidLocalFunctionId = -1; + static FunctionPtr LookupClosureFunction(const Function& member_function, - intptr_t kernel_offset); + intptr_t local_function_id); static FunctionPtr LookupClosureFunctionLocked( const Function& member_function, - intptr_t kernel_offset); + intptr_t local_function_id); // Normally implicit closure functions are not added to this cache, however // during AOT compilation we might add those implicit closure functions @@ -45,6 +47,7 @@ class ClosureFunctionsCache : public AllStatic { // discover them. static void AddClosureFunctionLocked( const Function& function, + intptr_t local_function_id, bool allow_implicit_closure_functions = false); static intptr_t FindClosureIndex(const Function& needle); diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index 23f2fe152cb..4e801df4c35 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/runtime/vm/compiler/aot/precompiler.cc @@ -2249,7 +2249,8 @@ void Precompiler::DropFunctions() { implicit_closure = function.ImplicitClosureFunction(); RELEASE_ASSERT(functions_to_retain_.ContainsKey(implicit_closure)); ClosureFunctionsCache::AddClosureFunctionLocked( - implicit_closure, /*allow_implicit_closure_functions=*/true); + implicit_closure, ClosureFunctionsCache::kInvalidLocalFunctionId, + /*allow_implicit_closure_functions=*/true); } dropped_function_count_++; if (FLAG_trace_precompiler) { diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index 5176bcccf11..2fdbfb9a77e 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -3043,14 +3043,13 @@ Fragment StreamingFlowGraphBuilder::BuildLocalFunctionInvocation( AlternativeReadingScope alt( &reader_, variable_kernel_position - data_program_offset_); SkipVariableDeclaration(); - ReadUInt(); // read id. - // FunctionNode follows the variable declaration. - const intptr_t function_node_kernel_offset = ReaderOffset(); + const intptr_t local_function_id = ReadUInt(); // read id. + ASSERT(local_function_id > 0); target_function = ClosureFunctionsCache::LookupClosureFunction( Function::Handle(Z, parsed_function()->function().GetOutermostFunction()), - function_node_kernel_offset); + local_function_id); RELEASE_ASSERT(!target_function.IsNull()); } @@ -4311,10 +4310,11 @@ Fragment StreamingFlowGraphBuilder::BuildRecordFieldGet(TokenPosition* p, } Fragment StreamingFlowGraphBuilder::BuildFunctionExpression() { - const intptr_t offset = ReaderOffset() - 1; // Include the tag. - ReadPosition(); // read position. - ReadUInt(); // read id. - return BuildFunctionNode(offset); + const intptr_t offset = ReaderOffset() - 1; // Include the tag. + ReadPosition(); // read position. + const intptr_t local_function_id = ReadUInt(); // read id. + ASSERT(local_function_id > 0); + return BuildFunctionNode(local_function_id, offset); } Fragment StreamingFlowGraphBuilder::BuildLet(TokenPosition* p) { @@ -5939,23 +5939,25 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionDeclaration( const intptr_t variable_offset = ReaderOffset() + data_program_offset_; SkipVariableDeclaration(); - ReadUInt(); // read id. + const intptr_t local_function_id = ReadUInt(); // read id. + ASSERT(local_function_id > 0); Fragment instructions = DebugStepCheck(pos); - instructions += BuildFunctionNode(offset); + instructions += BuildFunctionNode(local_function_id, offset); instructions += StoreLocal(pos, LookupVariable(variable_offset)); instructions += Drop(); return instructions; } Fragment StreamingFlowGraphBuilder::BuildFunctionNode( + intptr_t local_function_id, intptr_t func_decl_offset) { const intptr_t func_node_offset = ReaderOffset(); const auto& member_function = Function::Handle(Z, parsed_function()->function().GetOutermostFunction()); const Function& function = Function::ZoneHandle( Z, KernelLoader::GetClosureFunction( - thread(), func_decl_offset, member_function, + thread(), local_function_id, func_decl_offset, member_function, parsed_function()->function(), closure_owner_)); if (function.context_scope() == ContextScope::null()) { diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h index 234d67a0093..9e903a9b7a0 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h @@ -372,7 +372,8 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper { Fragment BuildYieldStatement(TokenPosition* position); Fragment BuildVariableDeclaration(TokenPosition* position); Fragment BuildFunctionDeclaration(TokenPosition* position); - Fragment BuildFunctionNode(intptr_t func_decl_offset); + Fragment BuildFunctionNode(intptr_t local_function_id, + intptr_t func_decl_offset); // Build flow graph for '_nativeEffect'. Fragment BuildNativeEffect(); diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index b8249c32dfa..456fed60a2c 100644 --- a/runtime/vm/kernel_loader.cc +++ b/runtime/vm/kernel_loader.cc @@ -2245,7 +2245,8 @@ FunctionPtr KernelLoader::LoadClosureFunction(const Function& parent_function, name = &Symbols::AnonymousClosure(); } - helper_.ReadUInt(); // read id. + const intptr_t local_function_id = helper_.ReadUInt(); // read id. + ASSERT(local_function_id > 0); const intptr_t func_node_offset = helper_.ReaderOffset(); @@ -2310,19 +2311,28 @@ FunctionPtr KernelLoader::LoadClosureFunction(const Function& parent_function, signature ^= ClassFinalizer::FinalizeType(signature); function.SetSignature(signature); - ClosureFunctionsCache::AddClosureFunctionLocked(function); + ClosureFunctionsCache::AddClosureFunctionLocked(function, local_function_id); return function.ptr(); } FunctionPtr KernelLoader::GetClosureFunction(Thread* thread, + intptr_t local_function_id, intptr_t func_decl_offset, const Function& member_function, const Function& parent_function, const Object& closure_owner) { Zone* zone = thread->zone(); Function& function = Function::Handle(zone); - intptr_t func_node_offset = -1; + + { + SafepointReadRwLocker ml(thread, thread->isolate_group()->program_lock()); + function = ClosureFunctionsCache::LookupClosureFunctionLocked( + member_function, local_function_id); + if (!function.IsNull()) { + return function.ptr(); + } + } const auto& kernel_info = KernelProgramInfo::Handle(zone, member_function.KernelProgramInfo()); @@ -2333,26 +2343,9 @@ FunctionPtr KernelLoader::GetClosureFunction(Thread* thread, KernelLoader kernel_loader(kernel_info, library_kernel_data, library_kernel_offset); - { - // TODO(alexmarkov): Use func_decl_offset as a key in ClosureFunctionsCache - // instead of func_node_offset and avoid this reading. - kernel_loader.helper_.SetOffset(func_decl_offset); - kernel_loader.helper_.ReadUntilFunctionNode(); - func_node_offset = kernel_loader.helper_.ReaderOffset(); - - { - SafepointReadRwLocker ml(thread, thread->isolate_group()->program_lock()); - function = ClosureFunctionsCache::LookupClosureFunctionLocked( - member_function, func_node_offset); - if (!function.IsNull()) { - return function.ptr(); - } - } - } - SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); function = ClosureFunctionsCache::LookupClosureFunctionLocked( - member_function, func_node_offset); + member_function, local_function_id); if (function.IsNull()) { ActiveClassScope active_class_scope( &kernel_loader.active_class_, diff --git a/runtime/vm/kernel_loader.h b/runtime/vm/kernel_loader.h index 9fe11e0b95a..9ab092d0560 100644 --- a/runtime/vm/kernel_loader.h +++ b/runtime/vm/kernel_loader.h @@ -211,6 +211,7 @@ class KernelLoader : public ValueObject { // Get closure Function from cache or create it if it is not created yet. // [func_decl_offset] is an offset FunctionExpression or FunctionDeclaration. static FunctionPtr GetClosureFunction(Thread* thread, + intptr_t local_function_id, intptr_t func_decl_offset, const Function& member_function, const Function& parent_function, diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc index c2dec3111ab..054b20f6fe4 100644 --- a/runtime/vm/object_test.cc +++ b/runtime/vm/object_test.cc @@ -5350,11 +5350,11 @@ ISOLATE_UNIT_TEST_CASE(FindClosureIndex) { const String& function_name = String::Handle(Symbols::New(thread, "foo")); function = Function::NewClosureFunction(function_name, parent, TokenPosition::kMinSource); - function.set_kernel_offset(42); // Add closure function to class. { SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); - ClosureFunctionsCache::AddClosureFunctionLocked(function); + ClosureFunctionsCache::AddClosureFunctionLocked(function, + /*local_function_id=*/42); } // The closure should return a valid index.