kicad-developers team mailing list archive

[jp charras <jp.charras@xxxxxxxxxx>]

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

 eeschema/sch_component.cpp | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/eeschema/sch_component.cpp b/eeschema/sch_component.cpp
index d4c647e..761a710 100644
--- a/eeschema/sch_component.cpp
+++ b/eeschema/sch_component.cpp
@@ -285,17 +285,54 @@ bool SCH_COMPONENT::Resolve( PART_LIBS* aLibs )
     return false;
 }
 
+// Helper sort function, used in SCH_COMPONENT::ResolveAll, to sort
+// sch component by lib_id
+static bool sort_by_libid( const SCH_COMPONENT* ref, SCH_COMPONENT* cmp )
+{
+    return ref->GetLibId() < cmp->GetLibId();
+}
 
 void SCH_COMPONENT::ResolveAll(
         const SCH_COLLECTOR& aComponents, PART_LIBS* aLibs )
 {
+    // Usually, many components use the same part lib.
+    // to avoid too long calculation time the list of components is grouped
+    // and once the lib part is found for one member of a group, it is also
+    // set for all other members of this group
+    std::vector<SCH_COMPONENT*> cmp_list;
+
+    // build the cmp list.
     for( int i = 0;  i < aComponents.GetCount();  ++i )
     {
         SCH_COMPONENT* cmp = dynamic_cast<SCH_COMPONENT*>( aComponents[i] );
         wxASSERT( cmp );
 
         if( cmp )   // cmp == NULL should not occur.
-            cmp->Resolve( aLibs );
+            cmp_list.push_back( cmp );
+    }
+
+    // sort it by lib part. Cmp will be grouped by same lib part.
+    std::sort( cmp_list.begin(), cmp_list.end(), sort_by_libid );
+
+    LIB_ID curr_libid;
+
+    for( unsigned ii = 0; ii < cmp_list.size (); ++ii )
+    {
+        SCH_COMPONENT* cmp = cmp_list[ii];
+        curr_libid = cmp->m_lib_id;
+        cmp->Resolve( aLibs );
+
+        // Propagate the m_part pointer to other members using the same lib_id
+        for( unsigned jj = ii+1; jj < cmp_list.size (); ++jj )
+        {
+            SCH_COMPONENT* next_cmp = cmp_list[jj];
+
+            if( curr_libid != next_cmp->m_lib_id )
+                break;
+
+            next_cmp->m_part = cmp->m_part;
+            ii = jj;
+        }
     }
 }
 

 eeschema/project_rescue.cpp | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/eeschema/project_rescue.cpp b/eeschema/project_rescue.cpp
index 1ca4dc0..fb664fa 100644
--- a/eeschema/project_rescue.cpp
+++ b/eeschema/project_rescue.cpp
@@ -146,16 +146,23 @@ static bool insert_library( PROJECT *aProject, PART_LIB *aLibrary, size_t aIndex
 static void get_components( std::vector<SCH_COMPONENT*>& aComponents )
 {
     SCH_SCREENS screens;
+    std::map<LIB_ID, SCH_COMPONENT*> raw_list;
+
+    //Get the full list
     for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )
     {
         for( SCH_ITEM* item = screen->GetDrawItems(); item; item = item->Next() )
         {
             if( item->Type() != SCH_COMPONENT_T )
                 continue;
-            SCH_COMPONENT* component = dynamic_cast<SCH_COMPONENT*>( item );
-            aComponents.push_back( component );
+            SCH_COMPONENT* component = static_cast<SCH_COMPONENT*>( item );
+            raw_list[component->GetLibId()] = component;
         }
     }
+
+    // Build the list
+    for( const auto &pair : raw_list )
+        aComponents.push_back( pair.second );
 }