mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 08:32:43 +00:00 
			
		
		
		
	LibJS: Stop converting between Object <-> IteratorRecord all the time
This patch makes IteratorRecord an Object. Although it's not exposed to
author code, this does allow us to store it in a VM register.
Now that we can store it in a VM register, we don't need to convert it
back and forth between IteratorRecord and Object when accessing it from
bytecode.
The big win here is avoiding 3 [[Get]] accesses on every iteration step
of for..of loops. There are also a bunch of smaller efficiencies gained.
20% speed-up on this microbenchmark:
    function go(a) {
        for (const p of a) {
        }
    }
    const a = [];
    a.length = 1_000_000;
    go(a);
			
			
This commit is contained in:
		
							parent
							
								
									4966c083df
								
							
						
					
					
						commit
						4699c81fc1
					
				
					 23 changed files with 226 additions and 144 deletions
				
			
		|  | @ -1749,16 +1749,11 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod | |||
| 
 | ||||
|         // 5. Let iterator be iteratorRecord.[[Iterator]].
 | ||||
|         auto iterator_register = generator.allocate_register(); | ||||
|         auto iterator_identifier = generator.intern_identifier("iterator"); | ||||
|         generator.emit_get_by_id(iterator_identifier); | ||||
|         generator.emit<Bytecode::Op::Store>(iterator_register); | ||||
|         generator.emit<Bytecode::Op::GetObjectFromIteratorRecord>(iterator_register, iterator_record_register); | ||||
| 
 | ||||
|         // Cache iteratorRecord.[[NextMethod]] for use in step 7.a.i.
 | ||||
|         auto next_method_register = generator.allocate_register(); | ||||
|         auto next_method_identifier = generator.intern_identifier("next"); | ||||
|         generator.emit<Bytecode::Op::Load>(iterator_record_register); | ||||
|         generator.emit_get_by_id(next_method_identifier); | ||||
|         generator.emit<Bytecode::Op::Store>(next_method_register); | ||||
|         generator.emit<Bytecode::Op::GetNextMethodFromIteratorRecord>(next_method_register, iterator_record_register); | ||||
| 
 | ||||
|         // 6. Let received be NormalCompletion(undefined).
 | ||||
|         // See get_received_completion_type_and_value above.
 | ||||
|  |  | |||
|  | @ -654,31 +654,9 @@ ThrowCompletionOr<NonnullGCPtr<Object>> super_call_with_argument_array(VM& vm, V | |||
|     return result; | ||||
| } | ||||
| 
 | ||||
| // FIXME: Since the accumulator is a Value, we store an object there and have to convert back and forth between that an Iterator records. Not great.
 | ||||
| // Make sure to put this into the accumulator before the iterator object disappears from the stack to prevent the members from being GC'd.
 | ||||
| Object* iterator_to_object(VM& vm, IteratorRecord iterator) | ||||
| { | ||||
|     auto& realm = *vm.current_realm(); | ||||
|     auto object = Object::create(realm, nullptr); | ||||
|     object->define_direct_property(vm.names.iterator, iterator.iterator, 0); | ||||
|     object->define_direct_property(vm.names.next, iterator.next_method, 0); | ||||
|     object->define_direct_property(vm.names.done, Value(iterator.done), 0); | ||||
|     return object; | ||||
| } | ||||
| 
 | ||||
| IteratorRecord object_to_iterator(VM& vm, Object& object) | ||||
| { | ||||
|     return IteratorRecord { | ||||
|         .iterator = &MUST(object.get(vm.names.iterator)).as_object(), | ||||
|         .next_method = MUST(object.get(vm.names.next)), | ||||
|         .done = MUST(object.get(vm.names.done)).as_bool() | ||||
|     }; | ||||
| } | ||||
| 
 | ||||
| ThrowCompletionOr<NonnullGCPtr<Array>> iterator_to_array(VM& vm, Value iterator) | ||||
| { | ||||
|     auto iterator_object = TRY(iterator.to_object(vm)); | ||||
|     auto iterator_record = object_to_iterator(vm, iterator_object); | ||||
|     auto& iterator_record = verify_cast<IteratorRecord>(iterator.as_object()); | ||||
| 
 | ||||
|     auto array = MUST(Array::create(*vm.current_realm(), 0)); | ||||
|     size_t index = 0; | ||||
|  | @ -832,47 +810,42 @@ ThrowCompletionOr<Object*> get_object_property_iterator(VM& vm, Value value) | |||
|                 properties.set(move(property_key)); | ||||
|         } | ||||
|     } | ||||
|     IteratorRecord iterator { | ||||
|         .iterator = object, | ||||
|         .next_method = NativeFunction::create( | ||||
|             *vm.current_realm(), | ||||
|             [items = move(properties)](VM& vm) mutable -> ThrowCompletionOr<Value> { | ||||
|                 auto& realm = *vm.current_realm(); | ||||
|                 auto iterated_object_value = vm.this_value(); | ||||
|                 if (!iterated_object_value.is_object()) | ||||
|                     return vm.throw_completion<InternalError>("Invalid state for GetObjectPropertyIterator.next"sv); | ||||
| 
 | ||||
|                 auto& iterated_object = iterated_object_value.as_object(); | ||||
|                 auto result_object = Object::create(realm, nullptr); | ||||
|                 while (true) { | ||||
|                     if (items.is_empty()) { | ||||
|                         result_object->define_direct_property(vm.names.done, JS::Value(true), default_attributes); | ||||
|                         return result_object; | ||||
|                     } | ||||
| 
 | ||||
|                     auto key = items.take_first(); | ||||
| 
 | ||||
|                     // If the property is deleted, don't include it (invariant no. 2)
 | ||||
|                     if (!TRY(iterated_object.has_property(key))) | ||||
|                         continue; | ||||
| 
 | ||||
|                     result_object->define_direct_property(vm.names.done, JS::Value(false), default_attributes); | ||||
| 
 | ||||
|                     if (key.is_number()) | ||||
|                         result_object->define_direct_property(vm.names.value, PrimitiveString::create(vm, TRY_OR_THROW_OOM(vm, String::number(key.as_number()))), default_attributes); | ||||
|                     else if (key.is_string()) | ||||
|                         result_object->define_direct_property(vm.names.value, PrimitiveString::create(vm, key.as_string()), default_attributes); | ||||
|                     else | ||||
|                         VERIFY_NOT_REACHED(); // We should not have non-string/number keys.
 | ||||
|     auto& realm = *vm.current_realm(); | ||||
|     auto callback = NativeFunction::create( | ||||
|         *vm.current_realm(), [items = move(properties)](VM& vm) mutable -> ThrowCompletionOr<Value> { | ||||
|             auto& realm = *vm.current_realm(); | ||||
|             auto iterated_object_value = vm.this_value(); | ||||
|             if (!iterated_object_value.is_object()) | ||||
|                 return vm.throw_completion<InternalError>("Invalid state for GetObjectPropertyIterator.next"sv); | ||||
| 
 | ||||
|             auto& iterated_object = iterated_object_value.as_object(); | ||||
|             auto result_object = Object::create(realm, nullptr); | ||||
|             while (true) { | ||||
|                 if (items.is_empty()) { | ||||
|                     result_object->define_direct_property(vm.names.done, JS::Value(true), default_attributes); | ||||
|                     return result_object; | ||||
|                 } | ||||
|             }, | ||||
|             1, | ||||
|             vm.names.next), | ||||
|         .done = false, | ||||
|     }; | ||||
|     return iterator_to_object(vm, move(iterator)); | ||||
| 
 | ||||
|                 auto key = items.take_first(); | ||||
| 
 | ||||
|                 // If the property is deleted, don't include it (invariant no. 2)
 | ||||
|                 if (!TRY(iterated_object.has_property(key))) | ||||
|                     continue; | ||||
| 
 | ||||
|                 result_object->define_direct_property(vm.names.done, JS::Value(false), default_attributes); | ||||
| 
 | ||||
|                 if (key.is_number()) | ||||
|                     result_object->define_direct_property(vm.names.value, PrimitiveString::create(vm, TRY_OR_THROW_OOM(vm, String::number(key.as_number()))), default_attributes); | ||||
|                 else if (key.is_string()) | ||||
|                     result_object->define_direct_property(vm.names.value, PrimitiveString::create(vm, key.as_string()), default_attributes); | ||||
|                 else | ||||
|                     VERIFY_NOT_REACHED(); // We should not have non-string/number keys.
 | ||||
| 
 | ||||
|                 return result_object; | ||||
|             } | ||||
|         }, | ||||
|         1, vm.names.next); | ||||
|     return vm.heap().allocate<IteratorRecord>(realm, realm, object, callback, false).ptr(); | ||||
| } | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -35,8 +35,6 @@ MarkedVector<Value> argument_list_evaluation(VM&, Value arguments); | |||
| ThrowCompletionOr<void> create_variable(VM&, DeprecatedFlyString const& name, Op::EnvironmentMode, bool is_global, bool is_immutable, bool is_strict); | ||||
| ThrowCompletionOr<ECMAScriptFunctionObject*> new_class(VM&, Value super_class, ClassExpression const&, Optional<IdentifierTableIndex> const& lhs_name); | ||||
| ThrowCompletionOr<NonnullGCPtr<Object>> super_call_with_argument_array(VM&, Value argument_array, bool is_synthetic); | ||||
| Object* iterator_to_object(VM&, IteratorRecord); | ||||
| IteratorRecord object_to_iterator(VM&, Object&); | ||||
| ThrowCompletionOr<NonnullGCPtr<Array>> iterator_to_array(VM&, Value iterator); | ||||
| ThrowCompletionOr<void> append(VM& vm, Value lhs, Value rhs, bool is_spread); | ||||
| ThrowCompletionOr<Value> delete_by_id(Bytecode::Interpreter&, Value base, IdentifierTableIndex identifier); | ||||
|  |  | |||
|  | @ -46,8 +46,10 @@ | |||
|     O(GetByValueWithThis)              \ | ||||
|     O(GetCalleeAndThisFromEnvironment) \ | ||||
|     O(GetIterator)                     \ | ||||
|     O(GetObjectFromIteratorRecord)     \ | ||||
|     O(GetMethod)                       \ | ||||
|     O(GetNewTarget)                    \ | ||||
|     O(GetNextMethodFromIteratorRecord) \ | ||||
|     O(GetImportMeta)                   \ | ||||
|     O(GetObjectPropertyIterator)       \ | ||||
|     O(GetPrivateById)                  \ | ||||
|  |  | |||
|  | @ -1225,8 +1225,21 @@ ThrowCompletionOr<void> DeleteByValueWithThis::execute_impl(Bytecode::Interprete | |||
| ThrowCompletionOr<void> GetIterator::execute_impl(Bytecode::Interpreter& interpreter) const | ||||
| { | ||||
|     auto& vm = interpreter.vm(); | ||||
|     auto iterator = TRY(get_iterator(vm, interpreter.accumulator(), m_hint)); | ||||
|     interpreter.accumulator() = iterator_to_object(vm, iterator); | ||||
|     interpreter.accumulator() = TRY(get_iterator(vm, interpreter.accumulator(), m_hint)); | ||||
|     return {}; | ||||
| } | ||||
| 
 | ||||
| ThrowCompletionOr<void> GetObjectFromIteratorRecord::execute_impl(Bytecode::Interpreter& interpreter) const | ||||
| { | ||||
|     auto& iterator_record = verify_cast<IteratorRecord>(interpreter.reg(m_iterator_record).as_object()); | ||||
|     interpreter.reg(m_object) = iterator_record.iterator; | ||||
|     return {}; | ||||
| } | ||||
| 
 | ||||
| ThrowCompletionOr<void> GetNextMethodFromIteratorRecord::execute_impl(Bytecode::Interpreter& interpreter) const | ||||
| { | ||||
|     auto& iterator_record = verify_cast<IteratorRecord>(interpreter.reg(m_iterator_record).as_object()); | ||||
|     interpreter.reg(m_next_method) = iterator_record.next_method; | ||||
|     return {}; | ||||
| } | ||||
| 
 | ||||
|  | @ -1248,8 +1261,7 @@ ThrowCompletionOr<void> GetObjectPropertyIterator::execute_impl(Bytecode::Interp | |||
| ThrowCompletionOr<void> IteratorClose::execute_impl(Bytecode::Interpreter& interpreter) const | ||||
| { | ||||
|     auto& vm = interpreter.vm(); | ||||
|     auto iterator_object = TRY(interpreter.accumulator().to_object(vm)); | ||||
|     auto iterator = object_to_iterator(vm, iterator_object); | ||||
|     auto& iterator = verify_cast<IteratorRecord>(interpreter.accumulator().as_object()); | ||||
| 
 | ||||
|     // FIXME: Return the value of the resulting completion. (Note that m_completion_value can be empty!)
 | ||||
|     TRY(iterator_close(vm, iterator, Completion { m_completion_type, m_completion_value, {} })); | ||||
|  | @ -1259,8 +1271,7 @@ ThrowCompletionOr<void> IteratorClose::execute_impl(Bytecode::Interpreter& inter | |||
| ThrowCompletionOr<void> AsyncIteratorClose::execute_impl(Bytecode::Interpreter& interpreter) const | ||||
| { | ||||
|     auto& vm = interpreter.vm(); | ||||
|     auto iterator_object = TRY(interpreter.accumulator().to_object(vm)); | ||||
|     auto iterator = object_to_iterator(vm, iterator_object); | ||||
|     auto& iterator = verify_cast<IteratorRecord>(interpreter.accumulator().as_object()); | ||||
| 
 | ||||
|     // FIXME: Return the value of the resulting completion. (Note that m_completion_value can be empty!)
 | ||||
|     TRY(async_iterator_close(vm, iterator, Completion { m_completion_type, m_completion_value, {} })); | ||||
|  | @ -1270,8 +1281,7 @@ ThrowCompletionOr<void> AsyncIteratorClose::execute_impl(Bytecode::Interpreter& | |||
| ThrowCompletionOr<void> IteratorNext::execute_impl(Bytecode::Interpreter& interpreter) const | ||||
| { | ||||
|     auto& vm = interpreter.vm(); | ||||
|     auto iterator_object = TRY(interpreter.accumulator().to_object(vm)); | ||||
|     auto iterator = object_to_iterator(vm, iterator_object); | ||||
|     auto& iterator = verify_cast<IteratorRecord>(interpreter.accumulator().as_object()); | ||||
| 
 | ||||
|     interpreter.accumulator() = TRY(iterator_next(vm, iterator)); | ||||
|     return {}; | ||||
|  | @ -1818,4 +1828,14 @@ DeprecatedString Catch::to_deprecated_string_impl(Bytecode::Executable const&) c | |||
|     return "Catch"sv; | ||||
| } | ||||
| 
 | ||||
| DeprecatedString GetObjectFromIteratorRecord::to_deprecated_string_impl(Bytecode::Executable const&) const | ||||
| { | ||||
|     return DeprecatedString::formatted("GetObjectFromIteratorRecord object:{} <- iterator_record:{}", m_object, m_iterator_record); | ||||
| } | ||||
| 
 | ||||
| DeprecatedString GetNextMethodFromIteratorRecord::to_deprecated_string_impl(Bytecode::Executable const&) const | ||||
| { | ||||
|     return DeprecatedString::formatted("GetNextMethodFromIteratorRecord next_method:{} <- iterator_record:{}", m_next_method, m_iterator_record); | ||||
| } | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -1373,6 +1373,46 @@ private: | |||
|     IteratorHint m_hint { IteratorHint::Sync }; | ||||
| }; | ||||
| 
 | ||||
| class GetObjectFromIteratorRecord final : public Instruction { | ||||
| public: | ||||
|     GetObjectFromIteratorRecord(Register object, Register iterator_record) | ||||
|         : Instruction(Type::GetObjectFromIteratorRecord, sizeof(*this)) | ||||
|         , m_object(object) | ||||
|         , m_iterator_record(iterator_record) | ||||
|     { | ||||
|     } | ||||
| 
 | ||||
|     ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const; | ||||
|     DeprecatedString to_deprecated_string_impl(Bytecode::Executable const&) const; | ||||
| 
 | ||||
|     Register object() const { return m_object; } | ||||
|     Register iterator_record() const { return m_iterator_record; } | ||||
| 
 | ||||
| private: | ||||
|     Register m_object; | ||||
|     Register m_iterator_record; | ||||
| }; | ||||
| 
 | ||||
| class GetNextMethodFromIteratorRecord final : public Instruction { | ||||
| public: | ||||
|     GetNextMethodFromIteratorRecord(Register next_method, Register iterator_record) | ||||
|         : Instruction(Type::GetNextMethodFromIteratorRecord, sizeof(*this)) | ||||
|         , m_next_method(next_method) | ||||
|         , m_iterator_record(iterator_record) | ||||
|     { | ||||
|     } | ||||
| 
 | ||||
|     ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const; | ||||
|     DeprecatedString to_deprecated_string_impl(Bytecode::Executable const&) const; | ||||
| 
 | ||||
|     Register next_method() const { return m_next_method; } | ||||
|     Register iterator_record() const { return m_iterator_record; } | ||||
| 
 | ||||
| private: | ||||
|     Register m_next_method; | ||||
|     Register m_iterator_record; | ||||
| }; | ||||
| 
 | ||||
| class GetMethod final : public Instruction { | ||||
| public: | ||||
|     GetMethod(IdentifierTableIndex property) | ||||
|  | @ -1551,7 +1591,6 @@ public: | |||
| private: | ||||
|     size_t m_index; | ||||
| }; | ||||
| 
 | ||||
| } | ||||
| 
 | ||||
| namespace JS::Bytecode { | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling