Rather than MakeShareable(new Foo), prefer to use MakeShared<Foo>().
The difference is that the latter will allocate a single memory block to host both the reference counter and the shared object. The former will allocate the object first (new), then allocate the reference counter (MakeShareable).
This makes MakeShared faster than MakeShareable and helps reduce memory fragmentation when allocating/deallocating shared object frequently. In the lecture’s context, it won’t make any difference, but it’s better build the muscle memory
Indeed, like everything else. Shared pointers may not even be the right answer in some cases (extra memory cost of the counter, extra cost of incrementing/decrementing the counter, …) and unique pointer (TUniquePtr) or raw pointers might be better.
So yes, MakeShared might be a problem when using weak pointers. There might also be cases where using MakeShared is not even feasible and only MakeShareable works.
But worrying about the wasted memory because of long lived invalidated weak pointers is premature until you can actually measure a significant impact. For all you know, the wasted memory is insignificant compared to the overall memory usage of your application. Or the weak pointers get destroyed quickly after the last shared pointer anyway. Or the cost of the double-allocation of MakeShareable overshadows the cost of the wasted memory (frequent allocation/deallocation of shared objects). Or the cost of the lack of cache locality (“variable–>counter–>object”, vs “variable–>counter+object”) is significant. Or the memory wasted due to fragmentation is more than that of the weak pointers. Or …
At the end of the day, it’s like code optimization: you shouldn’t manually do it until and if you can measure an impact.
Here, you’re better off using MakeShared first and foremost, and only resort to MakeShareable if you have actual/measurable problems. In most cases, it will lead to equal or better performances, and only in very specific scenarios will it lead to worse ones (and of those, only a few will actually be significantly impacted).
Yes, you bring very valid points. My intention was to add further clarification on how each one worked and why you would want to choose one or the other, rather than a golden rule of only using MakeShared.
It’s definitely better to avoid the 2nd allocation for cache and fragmentation reasons, but I believe its important for the reader to know that they may be leaking memory if you have long-lived weak pointers (for example a weak pointer on the player or a manager).
Both have their downsides and it’s a trade-off, however I agree that MakeShared should usually be preferred. In any case, switching between MakeShared and MakeShareable is just a matter of a single line since both yield a TSharedPtr.
Sure there are tradeoffs to using MakeShared over MakeShareable but in practice, those are relatively rarely an issue, and the choice not quite as clear cut as you made it sound (it’s not just, nor only, about weak pointers with big objects). So in my initial post, I chose to keep it simple, I didn’t want to make things more complicated for most just to help the very few who will have the problem and remember of those tradeoffs.
After further examination of internal UE code, I realized that our assumptions about the MakeShared vs MakeShareable performance implications are wrong. For accessing the object we don’t do an extra indirection in any of the cases, since a shared pointer always contains a pointer to the object.
template< class ObjectType, ESPMode InMode >
SharedPointerInternals::FSharedReferencer< Mode > SharedReferenceCount;
FSharedReferencer contains is a pointer to a base class TReferenceControllerBase* that is either:
This means that choosing between them is not as straight forward as you initially mentioned, there is a very small performance difference that will only happen on the creation of the pointer (two allocs vs one). However the points about weak pointers keeping the memory alive and memory fragmentation still remain.
Right, I was relying on the code you provided initially when I made that comment. It actually makes no sense to have an implementation with a double indirection, it’s unnecessary and costly .
That said, cache locality is still a possible issue. If one copies a shared pointer (the TSharedPtr itself, not the object (*1)) a lot (and thus both update the counter and access the object frequently), it’s better to have the object and the counter next to each other in memory/cache (*2).
(*1) and this is systematic when accessing a weak pointer since it needs to be copied into a TSharedPtr first before accessing the object.
(*2) but this too depends on the usage pattern. In some cases, it may be better to keep them separate so that the object doesn’t get cache-invalidated when the counter gets updated. (But in most it won’t make any measurable difference)
I don’t think that changes anything (*3). The “double indirection” was more of a cherry-on-top bonus. My main point was that weak pointers are uncommon compared to regular shared pointers. Long-lived (after object destruction) weak pointers even more so. Long-lived weak pointers with a lot of wasted memory (compared to overall app usage) even more-more so.
(*3) and, to be pedantic, my opinion is that it’s straight forward for the vast, vast majority of cases, and actually quite, quite complicated for remaining use cases, much more than just “your object is relatively large, and you think you might work with weak pointers”, to the point that you should use measurements/benchmarks for those instead of using back-of-the-envelope computation.
This means that choosing between them is not as straight forward as you initially mentioned
When we both thought that there was a double indirection, that was definitely a very strong factor, rather than the cherry on the top… Hence why I thought it was important to bring it up.
I don’t think you can easily assume that what is the weak pointer usage of the project, hence my initial points on “be careful if you have long lived weak pointers to big objects” as an if, since your memory will be sitting there without being able to be deallocated.
In any case, I have not been disagreeing with using MakeShared by default. My only point from the start has been to raise the implications, especially when working with weak pointers. I like your points on the cache implications for weak pointers though, I didn’t consider that