Please proceed with caution. I don't have any issue with renaming it to make the intent clear but thorough real world testing should be done before changing over to dynamic_cast<>.
On 4/13/2016 12:42 PM, Tomasz Wlostowski wrote: > 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 >> > > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : [email protected] > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
