Always use `String` as `StringName` backing internally. by Ivorforce · Pull Request #104985 · godotengine/godot

Ivorforce · 2025-04-03T20:25:34Z

Closes godotengine/godot-proposals#11517.

Benchmark

I benchmarked the launch of Godot of master vs this PR. There was a ~2% improvement (which is still within fluctuations, so no guarantees):

 ❯ hyperfine -m25 -iw1 "bin/godot.macos.editor.arm64.master --path /Users/lukas/dev/godot/empty-game --editor --quit" "bin/godot.macos.editor.arm64 --path /Users/lukas/dev/godot/empty-game --editor --quit"
Benchmark 1: bin/godot.macos.editor.arm64.master --path /Users/lukas/dev/godot/empty-game --editor --quit
  Time (mean ± σ):      3.407 s ±  0.184 s    [User: 2.486 s, System: 0.359 s]
  Range (min … max):    3.062 s …  3.845 s    25 runs

Benchmark 2: bin/godot.macos.editor.arm64 --path /Users/lukas/dev/godot/empty-game --editor --quit
  Time (mean ± σ):      3.343 s ±  0.186 s    [User: 2.470 s, System: 0.352 s]
  Range (min … max):    3.066 s …  3.688 s    25 runs

Summary
  bin/godot.macos.editor.arm64 --path /Users/lukas/dev/godot/empty-game --editor --quit ran
    1.02 ± 0.08 times faster than bin/godot.macos.editor.arm64.master --path /Users/lukas/dev/godot/empty-game --editor --quit

Allocation Tracking

I tracked persistent allocations during a normal editor run (/Users/lukas/dev/godot/godot/bin/godot.macos.editor.arm64 --path /Users/lukas/dev/godot/empty-game --editor), measured during the idle peak after launch, using Apple Instruments.

1092408576b / 1.02Gib on master
1095411792b / 1.02Gib on this pr

So, the difference seems to be around 3mb / 0.27% (additional cost in this PR vs master) in the editor.

Binary size

The binary size shrinks by 300kb (0.16%), at least for me.

Verdict

I don't think the RAM saved is worth the costs of having a split StringName backing.
It's simpler, and more predicable, to know that any StringName can convert to String instantly and without cost.

Additionally, operations like size() can be better inlined, making other operations generally faster. This is desirable especially because StringName is supposed to be fast, and it can be detrimental if some operations are considerably slower than String.

This fact is exploitable to make additional RAM and performance savings, especially for well-known strings. For example, it could be tested whether StringName referencing could be faster than String allocations (working towards interning). This would not be possible if StringName does not guarantee a String backing.

Repiteo

Would this deprecate StaticCString by extension, or does that still fill a niche?

Ivorforce · 2025-04-16T17:59:59Z

Yeah, StaticCString would no longer be needed. At least after cleaning up surrounding functions.

Repiteo

I considered asking that to be added to this, but perhaps that's best suited as its own PR. It'd certainly warrant a separate commit in any case. But I'd be fine integrating this as-is; your call

core/string/string_name.cpp

Repiteo

On second thought, we probably should tackle the StaticCString removal here too. I checked back on the core meeting discussion points I missed, and it mentioned this PR/proposal needing more justification—I'd say removal of the StaticCString paradigm & consequential simplification of StringName creation makes for a solid justification. Plus, we'd be able to ensure that the performance/size savings still apply; I have no reason to believe they would change, but making sure now is the better call

Ivorforce · 2025-04-17T15:30:10Z

On second thought, we probably should tackle the StaticCString removal here too. I checked back on the core meeting discussion points I missed, and it mentioned this PR/proposal needing more justification—I'd say removal of the StaticCString paradigm & consequential simplification of StringName creation makes for a solid justification.

I didn't at first, because I was afraid it would be a far more complex change. Turns out I just needed to search-and-replace StaticCString::create and _scs_create with StringName (and delete functions). Pretty simple!

Plus, we'd be able to ensure that the performance/size savings still apply; I have no reason to believe they would change, but making sure now is the better call

I don't think it should affect persistent allocations, because StaticCString is a transient object that doesn't affect allocations.

Anyway, I tested allocations again just to be sure, and found this time, this PR decreased RAM usage by 3mb. I'm guessing at this point the difference might be general fluctuations, and it might be indicative that the change isn't large enough to measure.

Edit: Here's a quick benchmark to show the difference in String conversions:

	StringName s1 = "_ready";
	StringName s2 = "Xready";

	{
		auto t0 = std::chrono::high_resolution_clock::now();
		for (int i = 0; i < 100000000; i ++) {
			// Test
			String s = s1;
		}
		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 < 100000000; i ++) {
			// Test
			String s = s2;
		}
		auto t1 = std::chrono::high_resolution_clock::now();
		std::cout << std::chrono::duration_cast<std::chrono::milliseconds>(t1 - t0).count() << "ms\n";
	}

Which prints on master: 3333ms 1402ms
In this PR, they're both ~1380ms.

Repiteo

Wow, that was pretty painless after all. I thought it'd need a whole other commit, but it absolutely functions as one.

A proper merge can wait on the next core meeting to verify that other members had their concerns addressed, but I'm satisfied with the results 👍

core/core_string_names.h

Calinou

Tested locally (rebased on top of master 9f03bbf), it works as expected.

Benchmark

PC specifications

CPU: Intel Core i9-13900K
GPU: NVIDIA GeForce RTX 4090
RAM: 64 GB (2×32 GB DDR5-5800 C30)
SSD: Solidigm P44 Pro 2 TB
OS: Linux (Fedora 41)

Using a release optimized export template (production=yes lto=full) on the 3D platformer demo.

❯ hyperfine -iw1 -m25 "bin/godot.linuxbsd.template_release.x86_64.master --path /home/hugo/Documents/Git/godotengine/godot-demo-projects/3d/platformer --quit" "bin/godot.linuxbsd.template_release.x86_64 --path /home/hugo/Documents/Git/godotengine/godot-demo-projects/3d/platformer --quit"
Benchmark 1: bin/godot.linuxbsd.template_release.x86_64.master --path /home/hugo/Documents/Git/godotengine/godot-demo-projects/3d/platformer --quit
  Time (mean ± σ):      1.739 s ±  0.033 s    [User: 0.337 s, System: 0.372 s]
  Range (min … max):    1.674 s …  1.796 s    25 runs
 
Benchmark 2: bin/godot.linuxbsd.template_release.x86_64 --path /home/hugo/Documents/Git/godotengine/godot-demo-projects/3d/platformer --quit
  Time (mean ± σ):      1.690 s ±  0.075 s    [User: 0.262 s, System: 0.361 s]
  Range (min … max):    1.379 s …  1.787 s    25 runs
 
Summary
  bin/godot.linuxbsd.template_release.x86_64 --path /home/hugo/Documents/Git/godotengine/godot-demo-projects/3d/platformer --quit ran
    1.03 ± 0.05 times faster than bin/godot.linuxbsd.template_release.x86_64.master --path /home/hugo/Documents/Git/godotengine/godot-demo-projects/3d/platformer --quit

Stripped binary size:

master: 71,962,520 bytes
This PR: 70,377,432 bytes (-1.58 MB)

Repiteo · 2025-04-23T17:11:56Z

Thanks!

Ivorforce added discussion topic:core performance labels Apr 3, 2025

Ivorforce added this to the 4.x milestone Apr 3, 2025

Ivorforce requested a review from a team as a code owner April 3, 2025 20:25

Ivorforce force-pushed the stringname-always-string branch from 6afd77a to eeed782 Compare April 3, 2025 21:59

Repiteo reviewed Apr 16, 2025

View reviewed changes

Repiteo approved these changes Apr 16, 2025

View reviewed changes

core/string/string_name.cpp Outdated Show resolved Hide resolved

Repiteo modified the milestones: 4.x, 4.5 Apr 16, 2025

Repiteo requested changes Apr 17, 2025

View reviewed changes

Ivorforce force-pushed the stringname-always-string branch from eeed782 to e689e1f Compare April 17, 2025 15:26

Ivorforce requested review from a team as code owners April 17, 2025 15:26

Repiteo approved these changes Apr 18, 2025

View reviewed changes

core/core_string_names.h Outdated Show resolved Hide resolved

Repiteo added the for pr meeting label Apr 18, 2025

Calinou approved these changes Apr 22, 2025

View reviewed changes

Always use String as StringName backing internally.

91fe434

Ivorforce force-pushed the stringname-always-string branch from e689e1f to 91fe434 Compare April 23, 2025 12:57

Repiteo removed the for pr meeting label Apr 23, 2025

Repiteo merged commit 841c29d into godotengine:master Apr 23, 2025
20 checks passed

Ivorforce deleted the stringname-always-string branch April 23, 2025 18:00

DeeJayLSP mentioned this pull request Apr 23, 2025

Use LocalVector on GDScriptInstance::members #105555

Open

Ivorforce mentioned this pull request Apr 23, 2025

Optimize StringName by storing Span<char> instead of const char * for static C strings. #100358

Closed

DeeJayLSP mentioned this pull request Apr 24, 2025

[Core] Optimize HashMap #90082

Closed

This was referenced Sep 18, 2025

Add GDExtension function to construct StringName directly from char* #78580

Merged

StringName: remove From impl for &'static CStr godot-rust/gdext#1316

Merged

dsnopek mentioned this pull request Sep 21, 2025

Deprecate gdextension_string_name_new_with_* #110744

Open

Bromeon mentioned this pull request Nov 25, 2025

Add StringName::chars() godot-rust/gdext#1419

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Always use `String` as `StringName` backing internally.#104985

Always use `String` as `StringName` backing internally.#104985
Repiteo merged 1 commit intogodotengine:masterfrom
Ivorforce:stringname-always-string

Ivorforce commented Apr 3, 2025 •

edited

Loading

Uh oh!

Repiteo left a comment

Uh oh!

Ivorforce commented Apr 16, 2025

Uh oh!

Repiteo left a comment

Uh oh!

Uh oh!

Repiteo left a comment

Uh oh!

Ivorforce commented Apr 17, 2025 •

edited

Loading

Uh oh!

Repiteo left a comment

Uh oh!

Uh oh!

Calinou left a comment

Uh oh!

Uh oh!

Repiteo commented Apr 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Ivorforce commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark

Allocation Tracking

Binary size

Verdict

Uh oh!

Repiteo left a comment

Choose a reason for hiding this comment

Uh oh!

Ivorforce commented Apr 16, 2025

Uh oh!

Repiteo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Repiteo left a comment

Choose a reason for hiding this comment

Uh oh!

Ivorforce commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Repiteo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Calinou left a comment

Choose a reason for hiding this comment

Benchmark

Uh oh!

Uh oh!

Repiteo commented Apr 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ivorforce commented Apr 3, 2025 •

edited

Loading

Ivorforce commented Apr 17, 2025 •

edited

Loading