[Insight-developers] STYLE: Un-necessary use of static_cast -- should we care?
Bradley Lowekamp
blowekamp at mail.nih.gov
Mon Jul 23 21:24:30 EDT 2012
Hello Kent,
First:
I have not really tried this but I just prototyped it and it could be useful if you want to try to track down the cases a little more programmatically, and care to mess with tr1 typetraits a little. Consider adding the following to itkMacro.h:
#include <tr1/type_traits>
#include <tr1/type_compare>
template < typename TOuttype, typename TIntype >
TOuttype itk_dynamic_cast( TIntype in )
{
static_assert( std::tr1::is_pointer<TOuttype>::value );
static_assert( std::tr1::is_base_of<TIntype, TOuttype>::value );
return dynamic_cast<TOuttype>( in );
}
#define dynamic_cast itk_dynamic_cast
Second:
While I am aware the use of dynamic_cast is not high performance it didn't bother me being used in for this case. So I modified the performance test Matt linked with the following additional cases:
res = 0;
start = getticks();
for (size_t i=0 ; i<N ; ++i) {
const char baseName[] = "IndexedDataObject";
std::ostringstream oss;
oss << baseName << N;
std::string s = oss.str();
res += s.size();
}
end = getticks();
timed = elapsed(end, start);
timings[17].push_back(timed);
res = 0;
start = getticks();
for (size_t i=0 ; i<N ; ++i) {
char buf[100];
sprintf(buf, "IndexedDataObject%u", static_cast<unsigned int>(N));
std::string s(buf);
res += s.size();
}
end = getticks();
timed = elapsed(end, start);
timings[18].push_back(timed);
These is based on code that is in the GetInput methods. This following is the results on my i7 laptop:
reinterpret_cast known-type: with relative 1.00, fastest 11686168.00, average 12120401.33, stddev 298867.57
virtual function + reinterpret_cast: with relative 1.78, fastest 21040604.00, average 21536555.33, stddev 464657.48
member variable + reinterpret_cast: with relative 1.10, fastest 13127058.00, average 13311370.00, stddev 227979.57
dynamic_cast same-type-base success: with relative 0.97, fastest 11514018.00, average 11717135.50, stddev 125301.84
dynamic_cast same-type-level1 success: with relative 2.94, fastest 34896278.00, average 35578390.00, stddev 393787.56
dynamic_cast same-type-level2 success: with relative 2.88, fastest 34403850.00, average 34912766.67, stddev 516410.80
dynamic_cast same-type-level3 success: with relative 2.84, fastest 33781284.00, average 34444748.67, stddev 593710.46
dynamic_cast level1-to-base success: with relative 1.05, fastest 12517034.00, average 12768522.83, stddev 322468.58
dynamic_cast level2-to-base success: with relative 1.01, fastest 12024095.00, average 12287761.83, stddev 245272.40
dynamic_cast level3-to-base success: with relative 1.03, fastest 12074146.00, average 12427744.83, stddev 386862.93
dynamic_cast level2-to-level1 success: with relative 3.28, fastest 39064702.00, average 39723110.50, stddev 601394.77
dynamic_cast level3-to-level1 success: with relative 3.93, fastest 46206012.00, average 47576171.83, stddev 1193834.23
dynamic_cast level3-to-level2 success: with relative 3.38, fastest 40250794.00, average 40943911.83, stddev 454434.63
dynamic_cast onebase-to-twobase fail: with relative 2.00, fastest 23874560.00, average 24276072.83, stddev 1132888.98
dynamic_cast onelevel1-to-twobase fail: with relative 2.57, fastest 30602545.00, average 31203997.50, stddev 527747.22
dynamic_cast onelevel2-to-twobase fail: with relative 3.08, fastest 37019228.00, average 37358361.00, stddev 269371.47
dynamic_cast onelevel3-to-twobase fail: with relative 3.74, fastest 44848942.00, average 45287361.83, stddev 584046.88
ostringstream: with relative 144.16, fastest 1705792767.00, average 1747226021.83, stddev 20540335.54
sprintf: with relative 47.74, fastest 575666370.00, average 578647874.17, stddev 3708284.83
Brad
On Jul 23, 2012, at 5:36 PM, Matt McCormick wrote:
> On Mon, Jul 23, 2012 at 9:22 PM, Williams, Norman K
> <norman-k-williams at uiowa.edu> wrote:
>> RE unnecessary casts: Personally, I think casts uglify code and should
>> only be there as a way for the programmer to communicate intention to the
>> compiler.
>>
>>
>> RE adding dynamic casts: I don't know what the real overhead of a
>> dynamic_cast vs a static_cast entails, and I suspect it varies depending
>> on what compiler you use. Determining if it is logically impossible for
>> dynamic_cast to fail isn't easy; and not all filters are written in such a
>> way that prevent assigning bad inputs, like adding an Image where a Mesh
>> is expected.
>>
>> I like Sean McBride's idea of a debug-only assert. In fact it might be
>> useful to use static_cast for release and dynamic_cast + assert(rval != 0)
>> for debug builds.
>
> Keep in mind that static_cast is done at compile time and dynamic_cast
> is done during runtime. My understanding is that dynamic_cast can be
> "expensive", and an assert is not saving anything relative to the cost
> of the dynamic_cast.
>
> Some performance numbers:
>
> http://tinodidriksen.com/2010/04/14/cpp-dynamic-cast-performance/
>
> Thanks,
> Matt
>
>>
>> On 7/23/12 3:58 PM, "Matt McCormick" <matt.mccormick at kitware.com> wrote:
>>
>>>>
>>>> So the question is this: do we keep static_cast where they aren't
>>>> needed?
>>>
>>> Since it is a matter of human-readability, something consistent is
>>> preferred, whatever that may be...
>>>
>>>> and should we add dynamic_cast in the frequent case where an input or
>>>> output of a filter is assumed to be of a particular type?
>>>>
>>>
>>> If it is not logically possible in the code for the dynamic_cast to
>>> fail, and given the GetInput/GetOutput consequences, avoiding
>>> unnecessary overhead seems best.
>>
>>
>>
>> ________________________________
>> Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged. If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited. Please reply to the sender that you have received the message in error, then delete it. Thank you.
>> ________________________________
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Kitware offers ITK Training Courses, for more information visit:
>> http://kitware.com/products/protraining.php
>>
>> Please keep messages on-topic and check the ITK FAQ at:
>> http://www.itk.org/Wiki/ITK_FAQ
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.itk.org/mailman/listinfo/insight-developers
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.php
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
More information about the Insight-developers
mailing list