[PATCH] D43430: Omit nullptr check for sufficiently simple delete-expressions

[Andrew Hunter via cfe-commits]

On Sat, Feb 17, 2018 at 1:41 AM Kim Gräsman via Phabricator <
reviews at reviews.llvm.org> wrote:

> kimgr added a comment.
>
> Peanut gallery observation: there was a discussion on the Boost list years
> and years ago where someone made the case that `if (x != nullptr) delete
> x;` was measurably faster than just calling `delete x;` I can't find it
> now, but I think it might have been in the context of their
> `checked_delete` library. Anyway, the reasoning was that with an external
> nullptr check, you'd pay for one comparison, but without it you'd always
> pay for a jump + a comparison. I suppose that only holds true for null
> pointers, for non-null pointers the extra check is just waste.
>
>
This is all correct, to some extent; that said, the net difference will be
a call to a reasonably hot function and an extra branch.  For grins, I
benchmarked the cost of ::operator delete (this is slightly challenging
because clang currently (mis?) compiles ::operator delete(nullptr) away,
but this is solvable...)  With our tcmalloc, the cost of deleting null is
about 8 cycles (compared to an empty loop.) (I don't really know how to
benchmark the version with an if around it, but if we assume that's free, 8
cycles is still very cheap.)


> It looks to me like the compiler inserts an external null check, and
> you're now removing it in select cases, did I understand that right?


Yes, precisely.


> I wonder if this could have negative effects for frequent deletion of
> nullptrs (e.g. a sometimes-allocated member of a heavily used value type).
>
>
In principle yes.  I would be very surprised if any binary deletes null
pointers often enough for this to be a significant concern (other than
statically-provable ones, which are still free.) You'd have to be doing it
a lot to notice those 8 cycles.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180218/2834fab1/attachment.html>