Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize String ==, ends_with and begins_with by using memcmp. #101493

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

Ivorforce
Copy link
Member

@Ivorforce Ivorforce commented Jan 13, 2025

This should behave identically as before.

Benchmark

The new implementation can be about 4x as fast as before:

	String s1 = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.";
	String s2 = s1;

	{
		auto t0 = std::chrono::high_resolution_clock::now();
		for (int i = 0; i < 10000000; i ++) {
			// Test
			bool b = s1 == s2;
		}
		auto t1 = std::chrono::high_resolution_clock::now();
		std::cout << std::chrono::duration_cast<std::chrono::milliseconds>(t1 - t0).count() << "ms\n";
	}
	{
		auto t0 = std::chrono::high_resolution_clock::now();
		for (int i = 0; i < 10000000; i ++) {
			// Test
			bool b = s1.begins_with(s2);
		}
		auto t1 = std::chrono::high_resolution_clock::now();
		std::cout << std::chrono::duration_cast<std::chrono::milliseconds>(t1 - t0).count() << "ms\n";
	}
	{
		auto t0 = std::chrono::high_resolution_clock::now();
		for (int i = 0; i < 10000000; i ++) {
			// Test
			bool b = s1.ends_with(s2);
		}
		auto t1 = std::chrono::high_resolution_clock::now();
		std::cout << std::chrono::duration_cast<std::chrono::milliseconds>(t1 - t0).count() << "ms\n";
	}

Notes

This PR only optimizes string to string comparisons. It does not optimize comparison to string literals (which are char[]).

Copy link
Contributor

@kiroxas kiroxas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, and the line count is very good !

@akien-mga akien-mga requested a review from bruvzg January 13, 2025 14:34
@JekSun97
Copy link
Contributor

I haven't looked at the GDScript implementation, but does this performance improvement also apply to GDScript syntax rule parsing?

@Ivorforce
Copy link
Member Author

Ivorforce commented Jan 13, 2025

I haven't looked at the GDScript implementation, but does this performance improvement also apply to GDScript syntax rule parsing?

I don't know for sure, but I think it's unlikely that it will observably affect it. The reason is that most C++ code uses string literals, which are char[] instead of char32_t[] and thus they can't benefit from this change.

Copy link
Member

@hpvb hpvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had some questions about the change from get_data() but because of the checks above length() != len and is_empty() this should be safe.

@hpvb hpvb modified the milestones: 4.x, 4.4 Jan 13, 2025
@akien-mga akien-mga merged commit df0a268 into godotengine:master Jan 13, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@Ivorforce Ivorforce deleted the string-equal-fast branch January 13, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants