JP, If Chris is OK with these changes, please commit them. I'm sure users will appreciate the speed up.
Thanks, Wayne On 3/9/2017 6:19 AM, Chris Pavlina wrote: > The RescueProject() optimization looks good to me. > > On Thu, Mar 09, 2017 at 08:59:33AM +0100, jp charras wrote: >> Le 08/03/2017 à 22:50, Chris Pavlina a écrit : >>> On Wed, Mar 08, 2017 at 04:33:06PM -0500, Wayne Stambaugh wrote: >>>> On 3/8/2017 4:08 PM, Chris Pavlina wrote: >>>>> That's why I submitted such a trivial patch to the list first, I figured >>>>> someone would say something ;) >>>>> >>>>> It does make sense to me, because the GUI is blocked. The busy cursor >>>>> says to me "yes, the GUI is supposed to be blocked right now, it's not >>>>> frozen". Even with a progress bar, it can seem unresponsive - >>>>> particularly if 1) the progress bar ends up obscured, as can happen with >>>>> 'weird' window managers sometimes, or 2) if a single library takes a >>>>> particularly long time to load, which I'm sure will only get worse if we >>>>> eventually allow loading them over the internet like for footprint libs. >>>> >>>> It's the old belt and suspender method. Users have got to quit using >>>> those 'weird' window managers. It causes way too much grief. >>> >>> I'm not sure displaying a busy cursor while the GUI is frozen is grief. >>> I've always considered it correct behavior, to be honest, regardless of >>> the presence or absence of a weird window manager. The busy cursor has >>> been used for _years_ to indicate a GUI that cannot be interacted with, >>> so users expect it - not displaying it sends conflicting messages; you >>> are saying by the absence of such a standard item that the GUI is ready >>> to use. >>> >>>> I'm >>>> surprised that a single library load takes long enough to need a busy >>>> cursor but I'm not opposed to the patch. Does the progress bar in >>>> wxWidgets have a continuous mode? That doesn't solve the hidden >>>> progress window issue though. >>> >>> There is a continuous mode, though I'm not sure what the benefit of >>> using it would be, and it's a bit strange (you have to "poke" it pretty >>> continuously or it freezes). I wouldn't use it. >>> >>> I'm not sure why library loading got so slow suddenly. I'd profile it >>> but I don't have time. >> >> I had a look at this. >> I am not sure the library loading itself is really slower. >> >> the loading time is mainly due to the call to >> SCH_EDIT_FRAME::RescueProject(). >> (previously: 0.5 seconds, now 14 seconds with my large schematic test, but >> with not a lot of libraries) >> >> I am thinking the root problem is SCH_COMPONENT::Resolve(), now much more >> time consuming. >> >> One other time consuming is SCH_COMPONENT::ResolveAll (previously: a few ms, >> now 1,4 s with this >> schematic) due to the same reason. >> >> Both methods are not optimized, and the call to Resolve() is made many times >> for the same lib id. >> >> I optimized SCH_COMPONENT::ResolveAll to call Resolve() only once by lib id >> (see the attached patch. >> (previously: 1.4 s and 0.14 s with fast_sch_component-ResolveAll attached >> patch) >> >> I don't know why Resolve() is now so time consuming. >> (A fast search algo could be interesting, but I don't think the previous >> method was optimized) >> >> SCH_EDIT_FRAME::RescueProject() could be also optimized to minimize the >> calculations: there are a >> lot of redundant calculations. >> This is the more time consuming, due to intensive and not optimized use of >> SCH_COMPONENT::Resolve(). >> >> As a proof, I attached a basic patch (fast_sch_project_rescue_stub) with >> optimized search. >> However this patch is only a proof, and cannot be used as this, because it >> searches for modified >> symbols, but does not modify all corresponding components in schematic, only >> one (but the missing >> feature is not time consuming). >> The calculation time was 13 s and 0.5s with this patch. >> (You also can disable the call to RescueProject() in Eeschema: this is an >> option) >> >> >> I can commit fast_sch_component-ResolveAll patch (If Wayne agrees) because >> it works fine for me. >> The other patch needs more work. >> >> -- >> Jean-Pierre CHARRAS > > > _______________________________________________ > 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
