Skip to content

Conversation

dementive
Copy link
Contributor

This removes (almost) all of the std::vector usage and replaces it with Godot's LocalVector.

There is still one place in classes/wrapped.cpp where std::vector is still used that I didn't want to mess with because it calls push_back during static initialization and the allocation function that LocalVector::push_back uses from the GDExtension API isn't registered yet. Reworking this to figure out a better time to call push_back didn't look too fun so I'm just going to leave it as is.

I tested the build time difference with ClangBuildAnalyzer and got these results:

With these changes:

Executed in 103.10 secs

**** Time summary:
Compilation (1025 times):
Parsing (frontend): 1640.2 s
Codegen & opts (backend): 190.6 s

Without these changes

Executed in 106.30 secs

**** Time summary:
Compilation (1025 times):
Parsing (frontend): 1682.0 s
Codegen & opts (backend): 184.8 s

Copy link
Member

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

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

Makes sense to me. I'm not aware of any risks / caveats of LocalVector over std::vector.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Since we have engine container templates in godot-cpp it makes sense to use them (most of the std:: usage predate addition of templates).

@dsnopek dsnopek added enhancement This is an enhancement on the current functionality cherrypick:4.4 cherrypick:4.5 labels Sep 15, 2025
@dsnopek dsnopek added this to the 4.x milestone Sep 15, 2025
@dsnopek dsnopek merged commit 0974e97 into godotengine:master Sep 16, 2025
16 checks passed
@dsnopek
Copy link
Collaborator

dsnopek commented Sep 16, 2025

Thanks!

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 20, 2025

Cherry-picked for 4.5 in PR #1870

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 20, 2025

Cherry-picked for 4.4 in PR #1871

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement This is an enhancement on the current functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants