kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

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     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>