Reduce and prevent unnecessary random-access to List

Random-access access to `List` when iterating is `O(n^2)` (`O(n)` when
accessing a single element)

* Removed subscript operator, in favor of a more explicit `get`
* Added conversion from `Iterator` to `ConstIterator`
* Remade existing operations into other solutions when applicable

core/debugger/debugger_marshalls.cpp

@@ -38,10 +38,10 @@
 Array DebuggerMarshalls::ScriptStackDump::serialize() {
 	Array arr;
 	arr.push_back(frames.size() * 3);
-	for (int i = 0; i < frames.size(); i++) {
-		arr.push_back(frames[i].file);
-		arr.push_back(frames[i].line);
-		arr.push_back(frames[i].func);
+	for (const ScriptLanguage::StackInfo &frame : frames) {
+		arr.push_back(frame.file);
+		arr.push_back(frame.line);
+		arr.push_back(frame.func);
 	}
 	return arr;
 }

core/debugger/remote_debugger.cpp

@@ -206,8 +206,7 @@ void RemoteDebugger::flush_output() {
 		Vector<String> joined_log_strings;
 		Vector<String> strings;
 		Vector<int> types;
-		for (int i = 0; i < output_strings.size(); i++) {
-			const OutputString &output_string = output_strings[i];
+		for (const OutputString &output_string : output_strings) {
 			if (output_string.type == MESSAGE_TYPE_ERROR) {
 				if (!joined_log_strings.is_empty()) {
 					strings.push_back(String("\n").join(joined_log_strings));

core/debugger/remote_debugger_peer.cpp

@@ -45,7 +45,7 @@ bool RemoteDebuggerPeerTCP::has_message() {
 Array RemoteDebuggerPeerTCP::get_message() {
 	MutexLock lock(mutex);
 	ERR_FAIL_COND_V(!has_message(), Array());
-	Array out = in_queue[0];
+	Array out = in_queue.front()->get();
 	in_queue.pop_front();
 	return out;
 }
@@ -100,7 +100,7 @@ void RemoteDebuggerPeerTCP::_write_out() {
 				break; // Nothing left to send
 			}
 			mutex.lock();
-			Variant var = out_queue[0];
+			Variant var = out_queue.front()->get();
 			out_queue.pop_front();
 			mutex.unlock();
 			int size = 0;

core/doc_data.cpp

@@ -152,9 +152,10 @@ void DocData::method_doc_from_methodinfo(DocData::MethodDoc &p_method, const Met
 
 	return_doc_from_retinfo(p_method, p_methodinfo.return_val);
 
-	for (int i = 0; i < p_methodinfo.arguments.size(); i++) {
+	int i = 0;
+	for (List<PropertyInfo>::ConstIterator itr = p_methodinfo.arguments.begin(); itr != p_methodinfo.arguments.end(); ++itr, ++i) {
 		DocData::ArgumentDoc argument;
-		argument_doc_from_arginfo(argument, p_methodinfo.arguments[i]);
+		argument_doc_from_arginfo(argument, *itr);
 		int default_arg_index = i - (p_methodinfo.arguments.size() - p_methodinfo.default_arguments.size());
 		if (default_arg_index >= 0) {
 			Variant default_arg = p_methodinfo.default_arguments[default_arg_index];

core/extension/extension_api_dump.cpp

@@ -1018,26 +1018,34 @@ Dictionary GDExtensionAPIDump::generate_extension_api(bool p_include_docs) {
 						d2["is_virtual"] = true;
 						// virtual functions have no hash since no MethodBind is involved
 						bool has_return = mi.return_val.type != Variant::NIL || (mi.return_val.usage & PROPERTY_USAGE_NIL_IS_VARIANT);
-						Array arguments;
-						for (int i = (has_return ? -1 : 0); i < mi.arguments.size(); i++) {
-							PropertyInfo pinfo = i == -1 ? mi.return_val : mi.arguments[i];
+						if (has_return) {
+							PropertyInfo pinfo = mi.return_val;
 							Dictionary d3;
 
-							if (i >= 0) {
-								d3["name"] = pinfo.name;
+							d3["type"] = get_property_info_type_name(pinfo);
+
+							if (mi.get_argument_meta(-1) > 0) {
+								d3["meta"] = get_type_meta_name((GodotTypeInfo::Metadata)mi.get_argument_meta(-1));
 							}
 
+							d2["return_value"] = d3;
+						}
+
+						Array arguments;
+						int i = 0;
+						for (List<PropertyInfo>::ConstIterator itr = mi.arguments.begin(); itr != mi.arguments.end(); ++itr, ++i) {
+							const PropertyInfo &pinfo = *itr;
+							Dictionary d3;
+
+							d3["name"] = pinfo.name;
+
 							d3["type"] = get_property_info_type_name(pinfo);
 
 							if (mi.get_argument_meta(i) > 0) {
 								d3["meta"] = get_type_meta_name((GodotTypeInfo::Metadata)mi.get_argument_meta(i));
 							}
 
-							if (i == -1) {
-								d2["return_value"] = d3;
-							} else {
-								arguments.push_back(d3);
-							}
+							arguments.push_back(d3);
 						}
 
 						if (arguments.size()) {
@@ -1151,10 +1159,11 @@ Dictionary GDExtensionAPIDump::generate_extension_api(bool p_include_docs) {
 
 					Array arguments;
 
-					for (int i = 0; i < F.arguments.size(); i++) {
+					int i = 0;
+					for (List<PropertyInfo>::ConstIterator itr = F.arguments.begin(); itr != F.arguments.end(); ++itr, ++i) {
 						Dictionary d3;
-						d3["name"] = F.arguments[i].name;
-						d3["type"] = get_property_info_type_name(F.arguments[i]);
+						d3["name"] = itr->name;
+						d3["type"] = get_property_info_type_name(*itr);
 						if (F.get_argument_meta(i) > 0) {
 							d3["meta"] = get_type_meta_name((GodotTypeInfo::Metadata)F.get_argument_meta(i));
 						}

core/extension/gdextension.cpp

@@ -211,14 +211,14 @@ class GDExtensionMethodBind : public MethodBind {
 		if (p_arg < 0) {
 			return return_value_info.type;
 		} else {
-			return arguments_info[p_arg].type;
+			return arguments_info.get(p_arg).type;
 		}
 	}
 	virtual PropertyInfo _gen_argument_type_info(int p_arg) const override {
 		if (p_arg < 0) {
 			return return_value_info;
 		} else {
-			return arguments_info[p_arg];
+			return arguments_info.get(p_arg);
 		}
 	}
 
@@ -232,7 +232,7 @@ class GDExtensionMethodBind : public MethodBind {
 		if (p_arg < 0) {
 			return return_value_metadata;
 		} else {
-			return arguments_metadata[p_arg];
+			return arguments_metadata.get(p_arg);
 		}
 	}
 #endif
@@ -319,8 +319,9 @@ class GDExtensionMethodBind : public MethodBind {
 			return false;
 		}
 
-		for (uint32_t i = 0; i < p_method_info->argument_count; i++) {
-			if (arguments_info[i].type != (Variant::Type)p_method_info->arguments_info[i].type) {
+		List<PropertyInfo>::ConstIterator itr = arguments_info.begin();
+		for (uint32_t i = 0; i < p_method_info->argument_count; ++itr, ++i) {
+			if (itr->type != (Variant::Type)p_method_info->arguments_info[i].type) {
 				return false;
 			}
 		}

core/io/ip.cpp

@@ -148,8 +148,8 @@ PackedStringArray IP::resolve_hostname_addresses(const String &p_hostname, Type
 	resolver->mutex.unlock();
 
 	PackedStringArray result;
-	for (int i = 0; i < res.size(); ++i) {
-		result.push_back(String(res[i]));
+	for (const IPAddress &E : res) {
+		result.push_back(String(E));
 	}
 	return result;
 }
@@ -206,9 +206,9 @@ IPAddress IP::get_resolve_item_address(ResolverID p_id) const {
 
 	List<IPAddress> res = resolver->queue[p_id].response;
 
-	for (int i = 0; i < res.size(); ++i) {
-		if (res[i].is_valid()) {
-			return res[i];
+	for (const IPAddress &E : res) {
+		if (E.is_valid()) {
+			return E;
 		}
 	}
 	return IPAddress();
@@ -226,9 +226,9 @@ Array IP::get_resolve_item_addresses(ResolverID p_id) const {
 	List<IPAddress> res = resolver->queue[p_id].response;
 
 	Array result;
-	for (int i = 0; i < res.size(); ++i) {
-		if (res[i].is_valid()) {
-			result.push_back(String(res[i]));
+	for (const IPAddress &E : res) {
+		if (E.is_valid()) {
+			result.push_back(String(E));
 		}
 	}
 	return result;

core/io/udp_server.cpp

@@ -161,7 +161,7 @@ Ref<PacketPeerUDP> UDPServer::take_connection() {
 		return conn;
 	}
 
-	Peer peer = pending[0];
+	Peer peer = pending.front()->get();
 	pending.pop_front();
 	peers.push_back(peer);
 	return peer.peer;

core/object/class_db.cpp

@@ -425,8 +425,8 @@ uint32_t ClassDB::get_api_hash(APIType p_api) {
 			for (const StringName &F : snames) {
 				MethodInfo &mi = t->signal_map[F];
 				hash = hash_murmur3_one_64(F.hash(), hash);
-				for (int i = 0; i < mi.arguments.size(); i++) {
-					hash = hash_murmur3_one_64(mi.arguments[i].type, hash);
+				for (const PropertyInfo &pi : mi.arguments) {
+					hash = hash_murmur3_one_64(pi.type, hash);
 				}
 			}
 		}
@@ -1856,8 +1856,9 @@ void ClassDB::add_virtual_method(const StringName &p_class, const MethodInfo &p_
 		if (p_arg_names.size() != mi.arguments.size()) {
 			WARN_PRINT("Mismatch argument name count for virtual method: " + String(p_class) + "::" + p_method.name);
 		} else {
-			for (int i = 0; i < p_arg_names.size(); i++) {
-				mi.arguments[i].name = p_arg_names[i];
+			List<PropertyInfo>::Iterator itr = mi.arguments.begin();
+			for (int i = 0; i < p_arg_names.size(); ++itr, ++i) {
+				itr->name = p_arg_names[i];
 			}
 		}
 	}

core/object/method_bind.h

@@ -152,7 +152,7 @@ class MethodBindVarArgBase : public MethodBind {
 		if (p_arg < 0) {
 			return _gen_return_type_info();
 		} else if (p_arg < method_info.arguments.size()) {
-			return method_info.arguments[p_arg];
+			return method_info.arguments.get(p_arg);
 		} else {
 			return PropertyInfo(Variant::NIL, "arg_" + itos(p_arg), PROPERTY_HINT_NONE, String(), PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_NIL_IS_VARIANT);
 		}
@@ -193,10 +193,11 @@ class MethodBindVarArgBase : public MethodBind {
 			Vector<StringName> names;
 			names.resize(method_info.arguments.size());
 #endif
-			for (int i = 0; i < method_info.arguments.size(); i++) {
-				at[i + 1] = method_info.arguments[i].type;
+			int i = 0;
+			for (List<PropertyInfo>::ConstIterator itr = method_info.arguments.begin(); itr != method_info.arguments.end(); ++itr, ++i) {
+				at[i + 1] = itr->type;
 #ifdef DEBUG_METHODS_ENABLED
-				names.write[i] = method_info.arguments[i].name;
+				names.write[i] = itr->name;
 #endif
 			}
 

core/object/object.cpp

@@ -1302,9 +1302,8 @@ TypedArray<Dictionary> Object::_get_signal_connection_list(const StringName &p_s
 
 TypedArray<Dictionary> Object::_get_incoming_connections() const {
 	TypedArray<Dictionary> ret;
-	int connections_amount = connections.size();
-	for (int idx_conn = 0; idx_conn < connections_amount; idx_conn++) {
-		ret.push_back(connections[idx_conn]);
+	for (const Object::Connection &connection : connections) {
+		ret.push_back(connection);
 	}
 
 	return ret;

core/templates/list.h

@@ -139,54 +139,58 @@ class List {
 
 	typedef T ValueType;
 
-	struct Iterator {
-		_FORCE_INLINE_ T &operator*() const {
+	struct ConstIterator {
+		_FORCE_INLINE_ const T &operator*() const {
 			return E->get();
 		}
-		_FORCE_INLINE_ T *operator->() const { return &E->get(); }
-		_FORCE_INLINE_ Iterator &operator++() {
+		_FORCE_INLINE_ const T *operator->() const { return &E->get(); }
+		_FORCE_INLINE_ ConstIterator &operator++() {
 			E = E->next();
 			return *this;
 		}
-		_FORCE_INLINE_ Iterator &operator--() {
+		_FORCE_INLINE_ ConstIterator &operator--() {
 			E = E->prev();
 			return *this;
 		}
 
-		_FORCE_INLINE_ bool operator==(const Iterator &b) const { return E == b.E; }
-		_FORCE_INLINE_ bool operator!=(const Iterator &b) const { return E != b.E; }
+		_FORCE_INLINE_ bool operator==(const ConstIterator &b) const { return E == b.E; }
+		_FORCE_INLINE_ bool operator!=(const ConstIterator &b) const { return E != b.E; }
 
-		Iterator(Element *p_E) { E = p_E; }
-		Iterator() {}
-		Iterator(const Iterator &p_it) { E = p_it.E; }
+		_FORCE_INLINE_ ConstIterator(const Element *p_E) { E = p_E; }
+		_FORCE_INLINE_ ConstIterator() {}
+		_FORCE_INLINE_ ConstIterator(const ConstIterator &p_it) { E = p_it.E; }
 
 	private:
-		Element *E = nullptr;
+		const Element *E = nullptr;
 	};
 
-	struct ConstIterator {
-		_FORCE_INLINE_ const T &operator*() const {
+	struct Iterator {
+		_FORCE_INLINE_ T &operator*() const {
 			return E->get();
 		}
-		_FORCE_INLINE_ const T *operator->() const { return &E->get(); }
-		_FORCE_INLINE_ ConstIterator &operator++() {
+		_FORCE_INLINE_ T *operator->() const { return &E->get(); }
+		_FORCE_INLINE_ Iterator &operator++() {
 			E = E->next();
 			return *this;
 		}
-		_FORCE_INLINE_ ConstIterator &operator--() {
+		_FORCE_INLINE_ Iterator &operator--() {
 			E = E->prev();
 			return *this;
 		}
 
-		_FORCE_INLINE_ bool operator==(const ConstIterator &b) const { return E == b.E; }
-		_FORCE_INLINE_ bool operator!=(const ConstIterator &b) const { return E != b.E; }
+		_FORCE_INLINE_ bool operator==(const Iterator &b) const { return E == b.E; }
+		_FORCE_INLINE_ bool operator!=(const Iterator &b) const { return E != b.E; }
 
-		_FORCE_INLINE_ ConstIterator(const Element *p_E) { E = p_E; }
-		_FORCE_INLINE_ ConstIterator() {}
-		_FORCE_INLINE_ ConstIterator(const ConstIterator &p_it) { E = p_it.E; }
+		Iterator(Element *p_E) { E = p_E; }
+		Iterator() {}
+		Iterator(const Iterator &p_it) { E = p_it.E; }
+
+		operator ConstIterator() const {
+			return ConstIterator(E);
+		}
 
 	private:
-		const Element *E = nullptr;
+		Element *E = nullptr;
 	};
 
 	_FORCE_INLINE_ Iterator begin() {
@@ -519,7 +523,9 @@ class List {
 		}
 	}
 
-	T &operator[](int p_index) {
+	// Random access to elements, use with care,
+	// do not use for iteration.
+	T &get(int p_index) {
 		CRASH_BAD_INDEX(p_index, size());
 
 		Element *I = front();
@@ -532,7 +538,9 @@ class List {
 		return I->get();
 	}
 
-	const T &operator[](int p_index) const {
+	// Random access to elements, use with care,
+	// do not use for iteration.
+	const T &get(int p_index) const {
 		CRASH_BAD_INDEX(p_index, size());
 
 		const Element *I = front();

drivers/unix/dir_access_unix.cpp

@@ -289,7 +289,7 @@ String DirAccessUnix::get_drive(int p_drive) {
 
 	ERR_FAIL_INDEX_V(p_drive, list.size(), "");
 
-	return list[p_drive];
+	return list.get(p_drive);
 }
 
 int DirAccessUnix::get_current_drive() {