kicad-developers team mailing list archive

[Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>]

On 13.04.2016 18:38, Chris Pavlina wrote:
> I would argue - quite strongly! - that it doesn't matter if it's faster in an
> isolated test if it isn't used enough to actually affect the overall code
> speed. Was this ever profiled in situ? This sort of thing just causes headaches
> when people misunderstand the usage and subtleties of the more limited
> "optimized" replacement. I find it hard to believe that we call dynamic_cast
> enough for it to be a performance issue.

Perhaps you're right, I'm not going to argue here.

>
> Also - *please* let me rename it. dynamic_cast vs dyn_cast is almost the
> _textboox_ example of poor naming. It's absolutely 100% non-obvious to someone
> reading code using the latter _why_ it was chosen, what its advantage is over
> the normal one, and whether it has any subtle issues that can cause bugs. At
> least it should be called something like dynamic_cast_fast so there's
> justification for its use in the actual code using it.

Go on with the renaming.

Tom

> 
> On Wed, Apr 13, 2016 at 06:34:28PM +0200, Tomasz Wlostowski wrote:
>> On 13.04.2016 18:19, Simon Richter wrote:
>>> Hi,
>>>
>>> On 13.04.2016 18:13, Chris Pavlina wrote:
>>>
>>>> What is the purpose of dyn_cast<> in include/core/typeinfo.h? Why don't we just
>>>> use dynamic_cast<>? And can we either replace the former with the latter, or
>>>> add a comment to the former explaining its purpose?
>>>
>>> It uses the parallel type system in EDA_ITEM rather than RTTI, so it
>>> works if RTTI is broken, e.g. when compiling with gcc 2.95.
>>>
>>
>> I wrote it inspired with Clang/LLVM design which uses a very similar
>> pattern. Sorry Simon, I didn't consider compatibility with gcc 2.95
>> would be of an advantage. My reasons were:
>>
>> 1) Much faster (code in the attachment):
>> - dynamic_cast<> : 9090437 usecs
>> - dyn_cast<> : 1832433 usecs (5x improvement)
>>
>> 2) Lightweight & compatible with existing Kicad type system.
>>
>> Cheers,
>> Tom
>