[jpc0]

I would strongly encourage you to reduce your use of std::shared_ptr in your code.

There is one instance where I saw you take a shared_ptr and then proceed to move from it. That could literally have been a unique_ptr and if you are making an assumption that that shared_ptr is still valid in another part of the codebase you at best now have UB but it is almost certainly a bug.

Just replace shared_ptr with unique_ptr, the advantage of managed languages is that you are in control of the memory, to then proceed to just use ref counting means you may as well have written the code in Java / C# since you incur all the same overhead of atomic references and don't get the advantages of a sophisticated GC.

unique_ptr can also trivially be upgraded to shared if you actually do need ref counting.

[victormeriqui]

Thanks for the feedback

I do have quite a lot of shared_ptrs throughout the entire code base, that could be reduced

The main reason behind that is I wanted a lot of flexibility between the components and didn't want to end up centralising too much logic in a single one

For example: there is a resource_manager which is a shared pointer created in the game component, it's shared with the scene_renderer (it needs to use it to get the resource data), but I like to keep it also in the game component for loading new resources

of course I could have it owned by just the scene_renderer and access it from there avoiding the reference counting

but this was a design decision that I stand behind as it really helped with clearer separation between components, and the performance, well let's say that the reference counting isn't the bottleneck here as using the console for output is pretty slow

Also the moving of shared pointers is just an optimization to avoid increasing and then immediately decreasing the ref count, no UB there since its passed by value in the argument, so it gets copied before being moved :)

[jpc0]

> Also the moving of shared pointers is just an optimization to avoid increasing and then immediately decreasing the ref count, no UB there since its passed by value in the argument, so it gets copied before being moved

I saw that, that function is probably completely inlined by the optimiser as well so likely the move doesn't even happen there.

Just wanted to make sure since I know a lot of devs not super proficient in C++ just sticks a shared pointer on things to get around worries about ownership and don't concider the tradeoffs.

For me I concider using a unique_ptr a form of compile time check for how I'm thinking about the code. I find shared_ptr to be a smell. It's not necessarily wrong but probably needs some reasoning about.

Another note, I saw some comments somewhere about SIMD vectorisation that needs to be implemented. I would check whether the compiler isn't already doing that and if it isn't I would see about changing the code to make if possible for the optimiser to generate vectorised code.

I still haven't been at a PC so haven't been able to properly look through the code but it is nice to at least see modern C++ being used in a codebase

[victormeriqui]

The compiler for sure does some vectorization for me currently, I would expect much worse performance otherwise, it's quite good at that

The SIMD idea there was more in the direction of structuring the data in a more vector friendly way, and then seeing if manually vectorized code would run better - but right now I haven't really invested much time into it :/

There's also multi threading which I wanted to add, by having the rasterizer handle triangles in parallel in different regions of the screen, but this is also just an idea which I haven't explored much yet

[ashleighz]

I don't necessarily agree. Using shared_ptr to not worry about lifetimes, a la C#, is not inherently a bad thing, not all code has to pretend to be Rust, especially a hobby project. From what I see, they're only really being used as dependency injection. So it has the benefits of the garbage collected model with none of the downsides

Also, speaking as a C++ game developer by profession, taking a shared_ptr copy and moving from it is a common way of saying "I will take a copy of this and keep it alive" whereas a shared_ptr ref only communicates that it can or might

[jpc0]

RAII using a unique_ptr in this case would have the benefits of a GC without any of the performance loss of an atomic or a lock depending on implementation.

Regarding "I will take a copy and ... keep it alive..." That is literally what the copy constructor of a sharedpointer does, you wouldn't need a move for that, the move literally just prevents the atomic increment.

I'm not saying there isn't a use case for shared_ptr, just that in this codebase at least in parts of it you could string replace with unique_ptr and get a free performance boost ( as trivial as it would be if the compiler hasn't already just done that ) because the atomic ref count is not needed.

OP did however clarify some design decisions on why the shared_ptr in some parts of the code and that is fine.

I stick by my statement. Stop using std::shared_ptr. Use std::unique_ptr and then convert to shared when it is actually necessary.

My point in C# was a mean spirited dig, but really though, you are giving up a ton of library features from C# by coding in C++ to then just treat it like C#. Write it in C# using the library that has already been optimised...

[mandarax8]

Agree on the over-use of shared_ptr (there is _a lot_ of copying shared ptrs going on here). But moving from the shared_ptr like this is quite idiomatic no?

  void Consol3Engine::RegisterFrameDrawer(std::shared_ptr<IFrameDrawer> frame_drawer)
  {
      frame_drawers.push_back(std::move(frame_drawer));
      ...
  }

[jpc0]

Its idiomatic sure but specifically with shared pointer a lot of time people are using shared pointers because they don't want to think about ownership, this turns out not to be the case here but think about it, the function is taking shared_ptr by value which increases the ref count when looked at from the signature.

In the calling code you then dereference that shared_ptr because it should still be valid but it isn't because it is now a moved from value.

I would in this specific case just replace shared_ptr with unique_ptr in the entire call stack up until here. If there isn't an implicit conversation here, put the conversation to shared in this function.

That makes it explicit ownership of the pointer is being moved into this function, it wasn't clear at all that that is happening with the shared_ptr.

EDIT:

You have to think of shared_ptr as global, if you moved from it ownership was changed but there is no way for everyone else holding a reference to thay shared pointer to know that.