TokisanGames / Terrain3D

A high performance, editable terrain system for Godot 4.
MIT License
2.22k stars 131 forks source link

Clean up Pass by reference & const correctness #410

Closed TokisanGames closed 4 months ago

TokisanGames commented 4 months ago

Description

Some functions pass various Godot objects by reference, others not. Here's a sizeof comparison, showing which objects are worth passing by reference. By reference passes a uint64_t 8 byte pointer.

Type, sizeof(type)

There are also many uses of const Ref<Terrain3DStorage> & in parameters, when the ref is already a reference pointer.

Tasks:

Started on a PR but then accidentally pushed it into main, so WIP at 24008a6137c38cd00156f3efbda9626413eae5a9

TokisanGames commented 4 months ago

@mihe I noticed all throughout Godot-jolt, you pass const RID&. But RID is already 8 bytes, and it copies over the same RID, so the use of & seems redundant. Is there another reason you use it?

mihe commented 4 months ago

The generated (C++) code for RID scared me enough to stop passing it by-value. I haven't looked at the generated assembly code for it, but seeing as how the copy constructor has to go through the bindings means that it'll never get inlined/optimized away, so struck me as quite expensive. Its move constructor is also not defined in the header file, so you're sort of relying on LTO/LTCG to optimize out that function call.

But for the most part I try to keep my codebase mostly Godot-like (with a lot of arbitrary exceptions) and Godot passes RID by-ref everywhere, presumably because of 32-bit platforms.

TokisanGames commented 4 months ago

Thanks @mihe