not_null<shared_ptr> overhead

[cconverse711]

What is the intended usage of not_null with std::shared_ptr? There is a significant overhead to wrapping std::shared_ptr in not_null because every access results in a costly copy of the shared_ptr. As not_null cannot be used with unique_ptr, it seems that not_null is not really usable with smart pointers in general.

[janwilmans]

@neilmacintosh can this issue #550, #415 and #89 be closed now that not_null<smartptr> works? or is the support still debated?

[asfernandes]

@neilmacintosh can this issue #550, #415 and #89 be closed now that not_null<smartptr> works? or is the support still debated?

Is the problem resolved or not?

[jwakely]

Using shared_ptr still results in lots of unnecessary copies, so I don't think this is resolved at all.

C++17 guaranteed elision helps with some of them, e.g. not_null<shared_ptr<T>>::operator-> will be less expensive, but operator* still makes an unnecessary copy by calling get() (apparently just to avoid repeating the Expects check in operator*).

[daschuer]

The issue is that in case of std:shared_ptr get() does a deep copy, because it is is_copy_constructible

GSL/include/gsl/pointers

Line 107 in 1683d87

constexpr std::conditional_t<std::is_copy_constructible<T>::value, T, const T&> get() const

can we do here a std::is_pod check instead?

[hsutter]

Thanks for pinging this old issue, sorry for the lag.

I agree that’s reasonable, and is_pod would be fine, though technically it's deprecated as of C++20 in favor of is_trivially copyable so perhaps use that.

FWIW, personally, if I were writing the code I’d write this general type alias (name can be bikeshedded; this is copied from cppfront where I use this for "in" parameters and it's called in<T>):

template<typename T>
using value_access_t =
    std::conditional_t <
        sizeof(T) < 2*sizeof(void*) && std::is_trivially_copy_constructible_v<T>,
        T const,
        T const&
    >;

which is generally useful, and change get()’s return from

std::conditional_t<std::is_copy_constructible<T>::value, T, const T&>

to

value_access_t<T>

Note this also solves the slight incongruence that right now the return value is sometimes const and sometimes not... here it's always const.

Caveat: I haven't tried it on not_null's .get().

What do you think?

[daschuer]

What is your reason for sizeof(T) < 2*sizeof(void*) ?
Or will just std::is_pointer work?

[hsutter]

@daschuer Good question!

The direct answer is "generality": The decision to return by value or by const& here felt the same to me as for "in" parameter passing. So I defaulted to using the same rule I use for that, which gives the right/expected result for raw pointers and shared_ptr, and didn't see a reason to invent a special rule.

The secondary answer is that I then saw doing it that way had (actual or potential) extra benefits:

• This way, as noted above, we got the const right for free. A minor improvement, but a nice one, and makes the use of the returned type more consistent.
• This way, other user-defined pointerlike types that are small and trivially copyable (maybe like std::experimental::observer_ptr? (*) ) will also naturally use the cheap copy -- if they happen to be indeed cheap to copy. We'll just do the right thing.

(*) I wonder if observer_ptr might go the same route as string_view... pre-C++23 string_view was not guaranteed to be trivially copyable, in C++23 it will be guaranteed to be trivially copyable. Right now observer_ptr is only required to be copyable, but not trivially copyable, so I wonder if it too will go through a similar evolution (if it's ever added to the standard), and this formulation will just do the right thing both ways as the type or its implementations may evolve.

[daschuer]

Ah, thank you for explaination.
So will you create a PR with this to solve the issue?

[beinhaerter]

What is your reason for sizeof(T) < 2*sizeof(void*) ?

I think Herb is following F.16: For “in” parameters, pass cheaply-copied types by value and others by reference to const.

From the Reason section:

What is “cheap to copy” depends on the machine architecture, but two or three words (doubles, pointers, references) are usually best passed by value.

From the Enforcement section:

(Simple) ((Foundation)) Warn when a parameter being passed by value has a size greater than 2 * sizeof(void*). Suggest using a reference to const instead.
(Simple) ((Foundation)) Warn when a parameter passed by reference to const has a size less than 2 * sizeof(void*). Suggest passing by value instead.

[dmitrykobets-msft]

@daschuer sorry for the delay - I'll make a PR to implement Herb's suggestion and resolve this issue.

[daschuer]

Cool, thank you.