-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Improve descriptions of LocalVector, recommend it when appropriate
#11142
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
Conversation
|
The main benefit of LocalVector, apart from no copy-on-write, is that a LocalVector keeps its allocated memory. So when you frequently edit and clear stuff and it is used internally only you want to use a LocalVector to not waste performance on all the frequent memory alloc. |
Will this be changed? Related: godotengine/godot#105928 (review) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the new wording over the old. LocalVector is preferable to Vector whenever it's not expected that it needs to be copied around. We might actually make LocalVector the default recommended 'vector' type in the future for this.
Edit: Also, this needs a rebase after #11188.
I updated the description to address this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new description is incorrect.
Both Vector and LocalVector grow exponentially, and 'reserve' memory when resizing. The capacity thing is a quirk of Vector and pertains only to removing elements, where Vector may reduce capacity automatically if it crosses a size border.
It is actually expected that vector types do not automatically shrink their capacity (at least in c++), so this is a caveat to Vector rather than an addendum to LocalVector.
If I would document this (not sure if I would because we are planning to change this soon) I would put it this way:
Note that Vector automatically reduces its capacity when removing elements, which may lead to reduced performance due to allocations when it grows again.
Meanwhile, the main reason that LocalVector is faster than Vector is the lack of copy-on-write. The capacity reduction stuff can be significant, but isn't in most situations. I would not mention this in LocalVector's description, rather something like:
Due to a lack of copy-on-write, LocalVector is usually faster than Vector. Prefer it over Vector when copying it cheaply is not needed.
Will this be changed? Related: godotengine/godot#105928 (review)
This may change for Vector in the 4.6 dev cycle, yes.
|
Merged. Thank you, and thanks for the reviews! |
The current documentation doesn't highlight the advantages of
LocalVectorand may lead to misunderstandings and discourage its use.In fact I rarely noticed the differences between
LocalVectorandVectorbefore, until seeing many PRs in 4.5 replacingVectorwithLocalVector. I think we should provide a clearer explanation ofLocalVectorin the documentation.