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

How to make handles to cube class both thread safe and efficient? #419

Open
IAmNotHanni opened this issue Jul 18, 2021 · 1 comment
Open
Labels
feat:concurrency multithreading, asynchronous events, concurrency feat:octree octree, cube computations org:discussion needs further discussion with others

Comments

@IAmNotHanni
Copy link
Member

IAmNotHanni commented Jul 18, 2021

We are passing around a lot of pointers, smart pointers or const references to cubes in our code. This is necessary, as it's a major part of our engine. For example I'm working on the new octree collision (see upcoming pull request later) which looks like this:

std::optional<RayCubeCollision<Cube>>
    ray_cube_collision_check(const std::shared_ptr<Cube> cube, const glm::vec3 pos,
    const glm::vec3 dir, const std::optional<std::uint32_t> grid_level_counter) 

So in this code I am passing the cube as std::shared_ptr. This is fine, but clang-tidy suggests to pass it as const reference to a shared pointer. Jason Turner and Scott Meyers however say that this is not a good idea.

We have the following options:

  • Pass it as const reference
  • Pass it as a raw pointer
  • Pass it as a std::shared_ptr
  • Pass it as a std::weak_ptr

Notes:

  • We do not want to transfer ownership of the cube, so passing by value and using std::move is not an option in my opinion.
  • Copying data could be costly
  • We need to copy the data though, because if we pass it as const reference, the data could become invalid while the function body is being executed.

I think this is a common question which will is relevant for many code parts.
What do you guys propose?

@IAmNotHanni
Copy link
Member Author

IAmNotHanni commented Jul 18, 2021

So... I guess passing it as std::shared_ptr is the best option?

@IAmNotHanni IAmNotHanni added the org:discussion needs further discussion with others label Jul 18, 2021
@IceflowRE IceflowRE added feat:concurrency multithreading, asynchronous events, concurrency feat:octree octree, cube computations labels Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat:concurrency multithreading, asynchronous events, concurrency feat:octree octree, cube computations org:discussion needs further discussion with others
Projects
None yet
Development

No branches or pull requests

2 participants