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

Improve ucaps.h _find_upper() and _find_lower() performance #93360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaronp64
Copy link
Contributor

Updated _find_upper() and _find_lower() to use a hashmap-like lookup instead of binary search. Reduces time of functions like String::findn(), String::to_upper(), and String::to_lower() by around 50-80% depending on strings used.

Compared using gdscript below:

extends Node2D

var text := "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."

func _ready() -> void:
	test_findn()
	test_to_lower()
	test_to_upper()

func test_findn():
	var ms_start = Time.get_ticks_msec()
	var to_find := "abc"
	for i in 1000000:
		text.findn(to_find)
	var ms_end = Time.get_ticks_msec()
	print("findn: " + str(ms_end - ms_start) + "ms")

func test_to_lower():
	var ms_start = Time.get_ticks_msec()
	for i in 1000000:
		text.to_lower()
	var ms_end = Time.get_ticks_msec()
	print("to_lower: " + str(ms_end - ms_start) + "ms")

func test_to_upper():
	var ms_start = Time.get_ticks_msec()
	for i in 1000000:
		text.to_upper()
	var ms_end = Time.get_ticks_msec()
	print("to_upper: " + str(ms_end - ms_start) + "ms")

Old:

findn: 13111ms
to_lower: 6793ms
to_upper: 5231ms

New:

findn: 2428ms
to_lower: 1435ms
to_upper: 2241ms

@AThousandShips
Copy link
Member

Updated _find_upper() and _find_lower() to use a hashmap-like lookup instead of binary search.  Reduces time of functions like String::findn(), String::to_upper(), and String::to_lower() by around 50-80% depending on strings used.
size_t size = 1;
while (size < count * 2) {
size *= 2;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

get_nearest_po2() * 2 for more speed? òwó

Copy link
Contributor Author

Choose a reason for hiding this comment

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

next_power_of_2() * 2 should work here, but would require changing next_power_of_2() to be constexpr. Size is only calculated once at compile time for each caps_table array, so speed shouldn't be an issue.

@Ivorforce
Copy link
Contributor

Ivorforce commented Jan 5, 2025

Just to state it here too: I have opened #99971, which yields better performance benefits for latin texts.
That's not to say there's no value in this PR! #99971 only optimizes latin (ascii+) texts. Texts with non-latin contents, like chinese, would benefit from this PR. But we should be aware of this caveat when considering merging this :)

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.

4 participants