AcademySoftwareFoundation / openvdb

OpenVDB - Sparse volume data structure and tools
http://www.openvdb.org/
Mozilla Public License 2.0
2.62k stars 647 forks source link

[BUG] Consider using `inline constexpr` for the large `sEdgeGroupTable` table defined in VolumeToMesh.h #1896

Open jessey-git opened 2 weeks ago

jessey-git commented 2 weeks ago

Environment

Operating System: Windows or Linux Version / Commit SHA: OpenVDB 11 Other: MSVC C++17 or g++

Describe the bug

There is a large 3.2kb table defined in VolumeToMesh.h that will be included in each translation unit that includes this header and the table will remain duplicated in the final executable. There's a few other smaller tables as well. https://github.com/AcademySoftwareFoundation/openvdb/blob/master/openvdb/openvdb/tools/VolumeToMesh.h#L441

The proper, ODR-safe, way to define such tables inside headers is to use inline constexpr with C++17 onwards.

To Reproduce

Using the const table:

g++ -O2 -std=c++17 main.cc impl_file1.cc impl_file2.cc
./a.out
main       : 0x555d077d0040
impl file 1: 0x555d077d0d40
impl file 2: 0x555d077d1a40

ls -lt ./a.out
-rwxr-xr-x 1 deadpin deadpin 24416 Sep 11 19:43 a.out

Expected behavior

Using the inline constexpr table:

g++ -O2 -std=c++17 main.cc impl_file1.cc impl_file2.cc
./a.out
main       : 0x55d94e584040
impl file 1: 0x55d94e584040
impl file 2: 0x55d94e584040

ls -lt ./a.out
-rwxr-xr-x 1 deadpin deadpin 20264 Sep 11 19:43 a.out

Additional context

(Add any other context about the problem here.)