Skip to content

Conversation

dementive
Copy link

@dementive dementive commented Sep 9, 2025

The <string> header was in class_db.hpp and method_bind.hpp even though no parts of it are used in either of those files. I also removed the <iostream> header from class_db.hpp because it was also unused. Both of these files ended up getting included in thousands of other cpp files so slowed down build time a lot.

There seem to be only 2 uses of code from <string> in godot-cpp as the code works now:

  1. In src/godot.cpp it was used for printf and snprintf. Removing from the class_db header broke this but just including stdio fixed it.
  2. It was also used in templates/char_traits.cpp for std::char_traits::length. I left the include in this file because I didn't want to change how it works and it's in a cpp file so it doesn't matter that much.

I did 2 test runs where I ran with ClangBuildAnalyzer and the time command then did scons --clean. These are the timings I got before and after making these changes:

Without these changes

Executed in 108.61 secs
Executed in 107.91 secs

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

With these changes

Executed in 97.04 secs
Executed in 91.19 secs

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

@dementive dementive requested a review from a team as a code owner September 9, 2025 02:02
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. We do not use std::string with godot-cpp. So it should be good as long as it compiles.

@Calinou Calinou added enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup labels Sep 9, 2025
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

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 topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants