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::chr to avoid calling strlen. Use String::chr instead of String(&chr, 1) where appropriate. #100314

Merged
merged 1 commit into from
Mar 13, 2025

Conversation

Ivorforce
Copy link
Member

@Ivorforce Ivorforce commented Dec 12, 2024

This is a small refactor to get rid of 3 uses of String(ptr, len) (which I'm planning to delete entirely, because it's inefficient). I also make String::chr inlineable because the function is trivial, and having it in the cpp file is unnecessary.

It's technically a tiny improvement too since the resulting code is faster and can be better optimized by the compiler. But it should be considered a refactor.

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.

Small one, but avoiding a useless function call is good.

…nstead of `String(&chr, 1)` where appropriate.
Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

Prerequisite to #104049, and makes complete sense in isolation.

@Repiteo Repiteo modified the milestones: 4.x, 4.5 Mar 13, 2025
@Repiteo Repiteo merged commit e97bb76 into godotengine:master Mar 13, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Mar 13, 2025

Thanks!

Comment on lines -1630 to -1631
char32_t c[2] = { p_char, 0 };
return String(c);
Copy link
Contributor

@Rindbee Rindbee Mar 14, 2025

Choose a reason for hiding this comment

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

This seems to be equivalent to

string.parse_utf32(Span<char32_t>(&p_char, 1));

rather than

string.parse_utf32(p_char);

Use 0 as the end mark.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly. The old implementation would have converted a NUL char to a zero-size string, while the new one will complain about it. This is also true for the string.parse_utf32(Span<char32_t>(&p_char, 1)); version.

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.

5 participants