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

Many class/struct member variables can be moved to initializer list #24242

Closed
Rubonnek opened this issue Dec 9, 2018 · 4 comments
Closed

Many class/struct member variables can be moved to initializer list #24242

Rubonnek opened this issue Dec 9, 2018 · 4 comments

Comments

@Rubonnek
Copy link
Member

Rubonnek commented Dec 9, 2018

Godot version:

master branch commit 41d1dba

Issue description:

There are many class/struct member variables that can be moved to an initializer to avoid creating objects twice.

The performance enhancement of doing this is negligible for primitive types and for compilers that already do this optimization by default (like GCC), but a performance improvement might be noticeable on compilers that do not do this since on some cases, generally speaking, creating non-primitive objects twice could hurt performance.

Here's a list of the member variables that can be moved. This file does not include the variables I moved in #24241. It's an easy patch for anyone who'd like to contribute.

Steps to reproduce:
To generate attached list in linux you must have cppcheck installed. Then:

  • cd $godot_repo
  • cppcheck --enable=performance --std=posix . 2> performance_check.txt
  • fgrep 'initialization list' performance_check.txt | fgrep -v '[thirdparty/'
@akien-mga akien-mga added this to the 3.1 milestone Dec 9, 2018
@akien-mga akien-mga modified the milestones: 3.1, 3.2 Dec 17, 2018
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Dec 13, 2019
@Cartogy
Copy link

Cartogy commented Jan 23, 2021

I'd like to try this.

@Calinou
Copy link
Member

Calinou commented Jan 23, 2021

@Cartogy Thanks for your interest in contributing 🙂

Since there's refactoring going on in the master branch (such as #43952), I'd wait until it's more stable.

@Cartogy
Copy link

Cartogy commented Jan 24, 2021

Understood. I will keep my eye on its progress.

@Rubonnek
Copy link
Member Author

Rubonnek commented Sep 17, 2021

Closing in favor tracking this issue in:

Edit: As a clarification -- there's a chance the performance enhancement of using an initializer list is already being done by the compiler in a certain optimization pass, and given the sheer amount of changes in the master branch, I don't think this is worth looking into. It's more important to keep an eye on uninitialized variables that could cause issues, which is what this issue implicitly tracks, but it's explicitly tracked by the aforementioned issue.

Perhaps it might be worth looking into non-primitive types into an initializer list after 4.0 is released, but not before, and even then it's only worth looking into if the construction of non-primitive types actually hurt performance in certain areas.

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

No branches or pull requests

5 participants