← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Fix some performance issues

 

Don't wait for my changes, I just wanted to feel out whether you'd be
willing to accept them soon. I'm not sure if or when I'll get to them.

On Tue, Jan 09, 2018 at 04:01:53PM -0500, Wayne Stambaugh wrote:
> Camille,
> 
> Although I didn't look over all of it, I'm OK with most of this since it
> is low risk but I wish you would have broken it into smaller patches.
> Reviewing a 124K patch is not my idea of fun.  I'm waiting on some file
> renaming work from Chris which I am sure is going to clash with your
> patch set.  As soon as Chris merges his changes, I will start merging
> your patches and ask you to rebase them as required.
> 
> Cheers,
> 
> Wayne
> 
> On 1/9/2018 3:41 PM, Camille 019 wrote:
> > Hi,
> > 
> > 
> > As promised, here are the patches up to date.
> > 
> > 
> > Regards,
> > 
> > Camille
> > 
> > ------------------------------------------------------------------------
> > *De :* Camille <camille019@xxxxxxxxxxx>
> > *Envoyé :* mardi 26 septembre 2017 21:56:07
> > *À :* Wayne Stambaugh
> > *Cc :* kicad-developers@xxxxxxxxxxxxxxxxxxx
> > *Objet :* Re: [Kicad-developers] [PATCH] Fix some performance issues
> >  
> > Hi,
> > 
> > Ok, I'll keep the patchset synchronized with the master branch, for
> > future inclusion. When will the feature freeze take effect ?
> > 
> > I agree my patchset has no real impact on the performance. I mainly
> > wanted to test clang-tidy on a consistent source code base. I also think
> > that keeping automated checks empty of warning can help to improve the
> > overall code quality.
> > 
> > Regards,
> > 
> > --
> > Camille
> > 
> > 
> > Le lun. 25 sept. 2017 à 16:21, Wayne Stambaugh <stambaughw@xxxxxxxxx> a
> > écrit :
> >> On 9/25/2017 4:07 AM, Maciej Sumiński wrote:
> >>
> >>     Hi Camille, I think the attached patches are sensible, even though
> >>     I hope most compilers apply optimizations to avoid performance
> >>     penalties whenever possible. 
> >>
> >> I agree although I'm not sure about the compiler optimization. A quick
> >> look at the patches and I'm not sure the performance gains will be
> >> that noticeable. Most of the changes are in code that is called once
> >> on demand in response to a user request as opposed to the drawing or
> >> drc code where large numbers of calls happen frequently. That being
> >> said, we really should be doing a better job of not passing entire
> >> objects on the stack unless there is a good reason to do so.
> >>
> >>     If the attached patches are autogenerated, then I would kindly ask
> >>     you to generate them when we reach the feature freeze stage. There
> >>     are a few branches that await to be merged and I am not sure
> >>     whether your patches would not create too many conflicts. During
> >>     the feature freeze we anticipate the code changes will not be as
> >>     disruptive as they are now. 
> >>
> >> I second this. There are quite a few outstanding patches that will
> >> likely have enough conflicts so I would rather not pile more on top of
> >> that.
> >>
> >>     Regards, Orson On 09/23/2017 12:17 PM, Camille 019 wrote:
> >>
> >>         Hi all, I recently play with clang-tidy on the KiCad source
> >>         code. Here is a set of 2 patch which fix the
> >>         unnecessary-copy-initialization<https://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-copy-initialization.html>
> >>         and
> >>         unnecessary-value-param<https://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-value-param.html>
> >>         checks. I will run more performance checks in the future. The
> >>         next two: -
> >>         for-range-copy<https://clang.llvm.org/extra/clang-tidy/checks/performance-for-range-copy.html>
> >>         -
> >>         type-promotion-in-math-fn<https://clang.llvm.org/extra/clang-tidy/checks/performance-type-promotion-in-math-fn.html>
> >>         Best regards, -- Camille
> >>         _______________________________________________ Mailing list:
> >>         https://launchpad.net/~kicad-developers Post to :
> >>         kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx> Unsubscribe :
> >>         https://launchpad.net/~kicad-developers More help :
> >>         https://help.launchpad.net/ListHelp 
> >>
> >>     _______________________________________________ Mailing list:
> >>     https://launchpad.net/~kicad-developers Post to :
> >>     kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx> Unsubscribe :
> >>     https://launchpad.net/~kicad-developers More help :
> >>     https://help.launchpad.net/ListHelp 
> >>
> >> _______________________________________________ Mailing list:
> >> https://launchpad.net/~kicad-developers Post to :
> >> kicad-developers@xxxxxxxxxxxxxxxxxxx
> >> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx> Unsubscribe :
> >> https://launchpad.net/~kicad-developers More help :
> >> https://help.launchpad.net/ListHelp
> > 
> > 
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers
> > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > More help   : https://help.launchpad.net/ListHelp
> > 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp


References