[Libreoffice-commits] core.git: 3 commits - comphelper/source include/comphelper

Michael Stahl mstahl at redhat.com
Mon Oct 21 11:11:44 PDT 2013


 comphelper/source/misc/accessibleeventnotifier.cxx |  153 ++++++++++++++-------
 include/comphelper/accessibleeventnotifier.hxx     |   32 ----
 2 files changed, 107 insertions(+), 78 deletions(-)

New commits:
commit ffd0e2911023e684ca1e206d18b45ef5aa6179f9
Author: Michael Stahl <mstahl at redhat.com>
Date:   Mon Oct 21 18:54:42 2013 +0200

    fdo#70465: speed up AccessibleEventNotifier::generateId()
    
    Iterating over all entries of a std::map is rather slow, so add a
    interval map to manage the free entries.
    
    A first attempt to use boost::icl::interval_set for this was abandoned;
    while the releaseId() function would be just 1 line, GCC 4.8.2 at least
    is unhappy about boost icl headers for non-obvious reasons:
    
    UnpackedTarball/boost/boost/icl/discrete_interval.hpp:45:225: error:
     default argument for template parameter for class enclosing ‘void
     boost::icl::boost_concept_check_dummy45(boost_concept_check45*)’
    
    Change-Id: I7b767aefee57df7743dc13a694b6a61abdd536c7

diff --git a/comphelper/source/misc/accessibleeventnotifier.cxx b/comphelper/source/misc/accessibleeventnotifier.cxx
index b938aa5..c46d54a 100644
--- a/comphelper/source/misc/accessibleeventnotifier.cxx
+++ b/comphelper/source/misc/accessibleeventnotifier.cxx
@@ -24,6 +24,7 @@
 #include <comphelper/guarding.hxx>
 
 #include <map>
+#include <limits>
 
 using namespace ::com::sun::star::uno;
 using namespace ::com::sun::star::lang;
@@ -43,46 +44,71 @@ namespace
                 ::cppu::OInterfaceContainerHelper*,
                 ::std::less< AccessibleEventNotifier::TClientId > > ClientMap;
 
+    /// key is the end of the interval, value is the start of the interval
+    typedef ::std::map<AccessibleEventNotifier::TClientId,
+                AccessibleEventNotifier::TClientId> IntervalMap;
+
     struct lclMutex
         : public rtl::Static< ::osl::Mutex, lclMutex > {};
     struct Clients
         : public rtl::Static< ClientMap, Clients > {};
+    struct FreeIntervals
+        : public rtl::StaticWithInit<IntervalMap, FreeIntervals> {
+            IntervalMap operator() () {
+                IntervalMap map;
+                map.insert(::std::make_pair(
+                    ::std::numeric_limits<AccessibleEventNotifier::TClientId>::max(), 1));
+                return map;
+            }
+        };
 
-    /// generates a new client id
-    static AccessibleEventNotifier::TClientId generateId()
+    static void releaseId(AccessibleEventNotifier::TClientId const nId)
     {
-        AccessibleEventNotifier::TClientId nBiggestUsedId = 0;
-        AccessibleEventNotifier::TClientId nFreeId = 0;
-
-        // look through all registered clients until we find a "gap" in the ids
-
-        // Note that the following relies on the fact the elements in the map
-        // are traveled with ascending keys (aka client ids)
-        ClientMap &rClients = Clients::get();
-        for (   ClientMap::const_iterator aLookup = rClients.begin();
-                aLookup != rClients.end();
-                ++aLookup
-            )
+        IntervalMap & rFreeIntervals(FreeIntervals::get());
+        IntervalMap::iterator const upper(rFreeIntervals.upper_bound(nId));
+        assert(upper != rFreeIntervals.end());
+        assert(nId < upper->second); // second is start of the interval!
+        if (nId + 1 == upper->second)
+        {
+            --upper->second; // add nId to existing interval
+        }
+        else
         {
-            AccessibleEventNotifier::TClientId nCurrent = aLookup->first;
-            OSL_ENSURE( nCurrent > nBiggestUsedId,
-                "AccessibleEventNotifier::generateId: "
-                "map is expected to be sorted ascending!" );
-
-            if ( nCurrent - nBiggestUsedId > 1 )
-            {   // found a "gap"
-                nFreeId = nBiggestUsedId + 1;
-                break;
+            IntervalMap::iterator const lower(rFreeIntervals.lower_bound(nId));
+            if (lower != rFreeIntervals.end() && lower->first == nId - 1)
+            {
+                // add nId by replacing lower with new merged entry
+                rFreeIntervals.insert(::std::make_pair(nId, lower->second));
+                rFreeIntervals.erase(lower);
+            }
+            else // otherwise just add new 1-element interval
+            {
+                rFreeIntervals.insert(::std::make_pair(nId, nId));
             }
-
-            nBiggestUsedId = nCurrent;
         }
+        // currently it's not checked whether intervals can be merged now
+        // hopefully that won't be a problem in practice
+    }
 
-        if ( !nFreeId )
-            nFreeId = nBiggestUsedId + 1;
+    /// generates a new client id
+    static AccessibleEventNotifier::TClientId generateId()
+    {
+        IntervalMap & rFreeIntervals(FreeIntervals::get());
+        assert(!rFreeIntervals.empty());
+        IntervalMap::iterator const iter(rFreeIntervals.begin());
+        AccessibleEventNotifier::TClientId const nFirst = iter->first;
+        AccessibleEventNotifier::TClientId const nFreeId = iter->second;
+        assert(nFreeId <= nFirst);
+        if (nFreeId != nFirst)
+        {
+            ++iter->second; // remove nFreeId from interval
+        }
+        else
+        {
+            rFreeIntervals.erase(iter); // remove 1-element interval
+        }
 
-        OSL_ENSURE( rClients.end() == rClients.find( nFreeId ),
-            "AccessibleEventNotifier::generateId: algorithm broken!" );
+        assert(Clients::get().end() == Clients::get().find(nFreeId));
 
         return nFreeId;
     }
@@ -158,6 +184,7 @@ namespace comphelper
         // remove it from the clients map
         delete aClientPos->second;
         Clients::get().erase( aClientPos );
+        releaseId(_nClient);
     }
 
     //---------------------------------------------------------------------
@@ -183,6 +210,7 @@ namespace comphelper
             // implementations have re-entrance problems and call into
             // revokeClient while we are notifying from here)
             Clients::get().erase(aClientPos);
+            releaseId(_nClient);
         }
 
         // notify the "disposing" event for this client
commit 493cd5268bb634ff717d8117e33ea7565321667e
Author: Michael Stahl <mstahl at redhat.com>
Date:   Mon Oct 21 15:51:58 2013 +0200

    remove pointless EventListeners typedef
    
    Change-Id: I9cb8ea746c87b394c4c5993de14f692ea018c558

diff --git a/comphelper/source/misc/accessibleeventnotifier.cxx b/comphelper/source/misc/accessibleeventnotifier.cxx
index cafe979..b938aa5 100644
--- a/comphelper/source/misc/accessibleeventnotifier.cxx
+++ b/comphelper/source/misc/accessibleeventnotifier.cxx
@@ -39,11 +39,10 @@ namespace
     typedef ::std::pair< AccessibleEventNotifier::TClientId,
             AccessibleEventObject > ClientEvent;
 
-    typedef ::cppu::OInterfaceContainerHelper EventListeners;
-    typedef ::std::map< AccessibleEventNotifier::TClientId, EventListeners*,
+    typedef ::std::map< AccessibleEventNotifier::TClientId,
+                ::cppu::OInterfaceContainerHelper*,
                 ::std::less< AccessibleEventNotifier::TClientId > > ClientMap;
 
-
     struct lclMutex
         : public rtl::Static< ::osl::Mutex, lclMutex > {};
     struct Clients
@@ -132,7 +131,8 @@ namespace comphelper
         TClientId nNewClientId = generateId( );
 
         // the event listeners for the new client
-        EventListeners* pNewListeners = new EventListeners( lclMutex::get() );
+        ::cppu::OInterfaceContainerHelper *const pNewListeners =
+            new ::cppu::OInterfaceContainerHelper( lclMutex::get() );
             // note that we're using our own mutex here, so the listener containers for all
             // our clients share this same mutex.
             // this is a reminiscense to the days where the notifier was asynchronous. Today this is
@@ -164,7 +164,7 @@ namespace comphelper
     void AccessibleEventNotifier::revokeClientNotifyDisposing( const TClientId _nClient,
             const Reference< XInterface >& _rxEventSource ) SAL_THROW( ( ) )
     {
-        EventListeners * pListeners(0);
+        ::cppu::OInterfaceContainerHelper* pListeners(0);
 
         {
             // rhbz#1001768 drop the mutex before calling disposeAndClear
commit 65fce1128cf99c91c9847d613f11fc9ea2325e08
Author: Michael Stahl <mstahl at redhat.com>
Date:   Mon Oct 21 15:49:20 2013 +0200

    AccessibleEventNotifier: remove implementation details from header
    
    Change-Id: Ia422df4066e77bbe3a43a380ba978815fe46dc9c

diff --git a/comphelper/source/misc/accessibleeventnotifier.cxx b/comphelper/source/misc/accessibleeventnotifier.cxx
index 14ac88c7..cafe979 100644
--- a/comphelper/source/misc/accessibleeventnotifier.cxx
+++ b/comphelper/source/misc/accessibleeventnotifier.cxx
@@ -20,8 +20,11 @@
 #include <comphelper/accessibleeventnotifier.hxx>
 #include <osl/diagnose.h>
 #include <rtl/instance.hxx>
+#include <cppuhelper/interfacecontainer.h>
 #include <comphelper/guarding.hxx>
 
+#include <map>
+
 using namespace ::com::sun::star::uno;
 using namespace ::com::sun::star::lang;
 using namespace ::com::sun::star::accessibility;
@@ -33,35 +36,39 @@ using namespace ::comphelper;
 //---------------------------------------------------------------------
 namespace
 {
+    typedef ::std::pair< AccessibleEventNotifier::TClientId,
+            AccessibleEventObject > ClientEvent;
+
+    typedef ::cppu::OInterfaceContainerHelper EventListeners;
+    typedef ::std::map< AccessibleEventNotifier::TClientId, EventListeners*,
+                ::std::less< AccessibleEventNotifier::TClientId > > ClientMap;
+
+
     struct lclMutex
         : public rtl::Static< ::osl::Mutex, lclMutex > {};
     struct Clients
-        : public rtl::Static< AccessibleEventNotifier::ClientMap, Clients > {};
-}
-
-//.........................................................................
-namespace comphelper
-{
-//.........................................................................
+        : public rtl::Static< ClientMap, Clients > {};
 
-    //---------------------------------------------------------------------
-    AccessibleEventNotifier::TClientId AccessibleEventNotifier::generateId()
+    /// generates a new client id
+    static AccessibleEventNotifier::TClientId generateId()
     {
-        TClientId nBiggestUsedId = 0;
-        TClientId nFreeId = 0;
+        AccessibleEventNotifier::TClientId nBiggestUsedId = 0;
+        AccessibleEventNotifier::TClientId nFreeId = 0;
 
         // look through all registered clients until we find a "gap" in the ids
 
-        // Note that the following relies on the fact the elements in the map are traveled with
-        // ascending keys (aka client ids)
-        AccessibleEventNotifier::ClientMap &rClients = Clients::get();
+        // Note that the following relies on the fact the elements in the map
+        // are traveled with ascending keys (aka client ids)
+        ClientMap &rClients = Clients::get();
         for (   ClientMap::const_iterator aLookup = rClients.begin();
                 aLookup != rClients.end();
                 ++aLookup
             )
         {
-            TClientId nCurrent = aLookup->first;
-            OSL_ENSURE( nCurrent > nBiggestUsedId, "AccessibleEventNotifier::generateId: map is expected to be sorted ascending!" );
+            AccessibleEventNotifier::TClientId nCurrent = aLookup->first;
+            OSL_ENSURE( nCurrent > nBiggestUsedId,
+                "AccessibleEventNotifier::generateId: "
+                "map is expected to be sorted ascending!" );
 
             if ( nCurrent - nBiggestUsedId > 1 )
             {   // found a "gap"
@@ -81,6 +88,41 @@ namespace comphelper
         return nFreeId;
     }
 
+    /** looks up a client in our client map, asserts if it cannot find it or
+        no event thread is present
+
+        @precond
+            to be called with our mutex locked
+
+        @param nClient
+            the id of the client to loopup
+        @param rPos
+            out-parameter for the position of the client in the client map
+
+        @return
+            <TRUE/> if and only if the client could be found and
+            <arg>rPos</arg> has been filled with it's position
+    */
+    static sal_Bool implLookupClient(
+            const AccessibleEventNotifier::TClientId nClient,
+            ClientMap::iterator& rPos )
+    {
+        // look up this client
+        ClientMap &rClients = Clients::get();
+        rPos = rClients.find( nClient );
+        OSL_ENSURE( rClients.end() != rPos,
+            "AccessibleEventNotifier::implLookupClient: invalid client id "
+            "(did you register your client?)!" );
+
+        return ( rClients.end() != rPos );
+    }
+}
+
+//.........................................................................
+namespace comphelper
+{
+//.........................................................................
+
     //---------------------------------------------------------------------
     AccessibleEventNotifier::TClientId AccessibleEventNotifier::registerClient( )
     {
@@ -104,17 +146,6 @@ namespace comphelper
     }
 
     //---------------------------------------------------------------------
-    sal_Bool AccessibleEventNotifier::implLookupClient( const TClientId _nClient, ClientMap::iterator& _rPos )
-    {
-        // look up this client
-        AccessibleEventNotifier::ClientMap &rClients = Clients::get();
-        _rPos = rClients.find( _nClient );
-        OSL_ENSURE( rClients.end() != _rPos, "AccessibleEventNotifier::implLookupClient: invalid client id (did you register your client?)!" );
-
-        return ( rClients.end() != _rPos );
-    }
-
-    //---------------------------------------------------------------------
     void AccessibleEventNotifier::revokeClient( const TClientId _nClient )
     {
         ::osl::MutexGuard aGuard( lclMutex::get() );
diff --git a/include/comphelper/accessibleeventnotifier.hxx b/include/comphelper/accessibleeventnotifier.hxx
index f0a745b..19d51aa 100644
--- a/include/comphelper/accessibleeventnotifier.hxx
+++ b/include/comphelper/accessibleeventnotifier.hxx
@@ -22,13 +22,8 @@
 
 #include <com/sun/star/accessibility/AccessibleEventObject.hpp>
 #include <com/sun/star/accessibility/XAccessibleEventListener.hpp>
-#include <osl/thread.hxx>
-#include <osl/conditn.hxx>
-#include <cppuhelper/interfacecontainer.h>
-#include "comphelper/comphelperdllapi.h"
 
-#include <map>
-#include <list>
+#include <comphelper/comphelperdllapi.h>
 
 //.........................................................................
 namespace comphelper
@@ -44,12 +39,6 @@ namespace comphelper
     public:
         typedef sal_uInt32  TClientId;
 
-        typedef ::std::pair< TClientId, ::com::sun::star::accessibility::AccessibleEventObject >
-                                                                                    ClientEvent;
-
-        typedef ::cppu::OInterfaceContainerHelper                                   EventListeners;
-        typedef ::std::map< TClientId, EventListeners*, ::std::less< TClientId > >  ClientMap;
-
     protected:
         AccessibleEventNotifier( );     // never implemented
         ~AccessibleEventNotifier( );    // never implemented
@@ -130,25 +119,6 @@ namespace comphelper
                         const ::com::sun::star::accessibility::AccessibleEventObject& _rEvent
                     ) SAL_THROW( ( ) );
 
-    private:
-        /// generates a new client id
-        COMPHELPER_DLLPRIVATE static    TClientId   generateId();
-
-        /** looks up a client in our client map, asserts if it cannot find it or no event thread is present
-
-            @precond
-                to be called with our mutex locked
-
-            @param _nClient
-                the id of the client to loopup
-            @param _rPos
-                out-parameter for the position of the client in the client map
-
-            @return
-                <TRUE/> if and only if the client could be found and <arg>_rPos</arg> has been filled with
-                it's position
-        */
-        COMPHELPER_DLLPRIVATE static    sal_Bool    implLookupClient( const TClientId _nClient, ClientMap::iterator& _rPos );
     };
 
 //.........................................................................


More information about the Libreoffice-commits mailing list