[ITK-dev] Fwd: Change in ITK[master]: BUG: Make valid calling with m_NumElements == 0

[Luc Hermitte]

Hi,

Le 26/10/2015 16:41, David Cole via Insight-developers a écrit :
> I would really really really like to see some evidence from you that
> adding nullptr checks is materially harmful to performance in some
> real world fill/copy scenario before agreeing to any patch which does
> not include nullptr checks.

Here are some micro-benchmarks (done with google-benchmark framework):

https://gist.github.com/LucHermitte/7f1b8d5dd61c861e6ef9

Adding the test around std::copy in the assignment operator doesn't
change anything. Nor the manual implementation.
(These are not real world scenarios, however the result would be the
same : no observable difference).

I still find that adding a redundant test, just to workaround the
implementation of a function in Debug mode and with a single family of
compiler, is a clumsy solution.

What the manual implementation does is as good as the defensive one.
However, we could loose potential optimizations (like memcpy being used
with POD types) when the size of the vector is big enough if the STL
implementation tries to do it.

BTW, memcpy(0,0,x) is officially an UB. And the more I read, the less
I'm convinced that func(nullptr, null_size) is officially defined
behaviour.

So, if you prefer a test, let's go for a test -- which should be
required only in the assignment operator.

Regards,

-- 
Luc Hermitte