comphelper/source/container/embeddedobjectcontainer.cxx | 91 +++++------ unotools/source/ucbhelper/XTempFile.hxx | 28 ++- unotools/source/ucbhelper/xtempfile.cxx | 129 ++++++++++++---- 3 files changed, 170 insertions(+), 78 deletions(-)
New commits: commit a1bba53d6fbc56537da0296c27db5cc443c8bca7 Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Fri Apr 5 15:37:50 2019 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Mon Apr 8 08:51:13 2019 +0200 tdf#117066 Saving ODT document with ~1500 bookmarks is slow, part 2 Individually, these don't make much difference, but they add up to a halving the time to save on my machine. OTempFileService was spending a lot of time in PropertySetMixin doing UNO reflection. Re-implement the required property interfaces directly.. Change-Id: I9b6ef439d4c56eb40c1f5d636e3e5cb888d5d4ff Reviewed-on: https://gerrit.libreoffice.org/70310 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/unotools/source/ucbhelper/XTempFile.hxx b/unotools/source/ucbhelper/XTempFile.hxx index 43150b675f07..8a3d9fd0be41 100644 --- a/unotools/source/ucbhelper/XTempFile.hxx +++ b/unotools/source/ucbhelper/XTempFile.hxx @@ -39,10 +39,12 @@ namespace utl { class TempFile; } typedef ::cppu::WeakImplHelper< css::io::XTempFile , css::io::XInputStream , css::io::XOutputStream - , css::io::XTruncate > OTempFileBase; + , css::io::XTruncate + , css::beans::XPropertySet + , css::beans::XFastPropertySet + , css::beans::XPropertyAccess > OTempFileBase; class OTempFileService : public OTempFileBase - , public ::cppu::PropertySetMixin< css::io::XTempFile > { protected: std::unique_ptr<utl::TempFile> mpTempFile; @@ -62,12 +64,6 @@ public: explicit OTempFileService (css::uno::Reference< css::uno::XComponentContext > const & context); //Methods - // XInterface - virtual css::uno::Any SAL_CALL queryInterface( const css::uno::Type& aType ) override; - virtual void SAL_CALL acquire( ) - throw () override; - virtual void SAL_CALL release( ) - throw () override; // XTypeProvider virtual css::uno::Sequence< css::uno::Type > SAL_CALL getTypes( ) override; @@ -97,6 +93,22 @@ public: // XTruncate virtual void SAL_CALL truncate() override; + // XPropertySet + virtual ::css::uno::Reference< ::css::beans::XPropertySetInfo > SAL_CALL getPropertySetInfo() override; + virtual void SAL_CALL setPropertyValue( const ::rtl::OUString& aPropertyName, const ::css::uno::Any& aValue ) override; + virtual ::css::uno::Any SAL_CALL getPropertyValue( const ::rtl::OUString& PropertyName ) override; + virtual void SAL_CALL addPropertyChangeListener( const ::rtl::OUString& aPropertyName, const ::css::uno::Reference< ::css::beans::XPropertyChangeListener >& xListener ) override; + virtual void SAL_CALL removePropertyChangeListener( const ::rtl::OUString& aPropertyName, const ::css::uno::Reference< ::css::beans::XPropertyChangeListener >& aListener ) override; + virtual void SAL_CALL addVetoableChangeListener( const ::rtl::OUString& PropertyName, const ::css::uno::Reference< ::css::beans::XVetoableChangeListener >& aListener ) override; + virtual void SAL_CALL removeVetoableChangeListener( const ::rtl::OUString& PropertyName, const ::css::uno::Reference< ::css::beans::XVetoableChangeListener >& aListener ) override; + // XFastPropertySet + virtual void SAL_CALL setFastPropertyValue( ::sal_Int32 nHandle, const ::css::uno::Any& aValue ) override; + virtual ::css::uno::Any SAL_CALL getFastPropertyValue( ::sal_Int32 nHandle ) override; + // XPropertyAccess + virtual ::css::uno::Sequence< ::css::beans::PropertyValue > SAL_CALL getPropertyValues() override; + virtual void SAL_CALL setPropertyValues( const ::css::uno::Sequence< ::css::beans::PropertyValue >& aProps ) override; + + virtual ~OTempFileService () override; }; #endif diff --git a/unotools/source/ucbhelper/xtempfile.cxx b/unotools/source/ucbhelper/xtempfile.cxx index a8e0771cbea9..dc05e4194237 100644 --- a/unotools/source/ucbhelper/xtempfile.cxx +++ b/unotools/source/ucbhelper/xtempfile.cxx @@ -21,6 +21,7 @@ #include <unotoolsservices.hxx> #include <com/sun/star/io/BufferSizeExceededException.hpp> #include <com/sun/star/io/NotConnectedException.hpp> +#include <com/sun/star/beans/PropertyAttribute.hpp> #include <cppuhelper/factory.hxx> #include <cppuhelper/supportsservice.hxx> #include <cppuhelper/typeprovider.hxx> @@ -28,13 +29,10 @@ #include <osl/file.hxx> #include <unotools/configmgr.hxx> #include <unotools/tempfile.hxx> +#include <cppuhelper/propshlp.hxx> -OTempFileService::OTempFileService(css::uno::Reference< css::uno::XComponentContext > const & context) -: ::cppu::PropertySetMixin< css::io::XTempFile >( - context - , static_cast< Implements >( IMPLEMENTS_PROPERTY_SET | IMPLEMENTS_FAST_PROPERTY_SET | IMPLEMENTS_PROPERTY_ACCESS ) - , css::uno::Sequence< OUString >() ) -, mpStream( nullptr ) +OTempFileService::OTempFileService(css::uno::Reference< css::uno::XComponentContext > const &) +: mpStream( nullptr ) , mbRemoveFile( true ) , mbInClosed( false ) , mbOutClosed( false ) @@ -50,26 +48,6 @@ OTempFileService::~OTempFileService () { } -// XInterface - -css::uno::Any SAL_CALL OTempFileService::queryInterface( css::uno::Type const & aType ) -{ - css::uno::Any aResult( OTempFileBase::queryInterface( aType ) ); - if (!aResult.hasValue()) - aResult = cppu::PropertySetMixin< css::io::XTempFile >::queryInterface( aType ); - return aResult; -}; -void SAL_CALL OTempFileService::acquire( ) -throw () -{ - OTempFileBase::acquire(); -} -void SAL_CALL OTempFileService::release( ) -throw () -{ - OTempFileBase::release(); -} - // XTypeProvider css::uno::Sequence< css::uno::Type > SAL_CALL OTempFileService::getTypes( ) @@ -365,6 +343,105 @@ void SAL_CALL OTempFileService::truncate() checkError(); } +#define PROPERTY_HANDLE_URI 1 +#define PROPERTY_HANDLE_REMOVE_FILE 2 +#define PROPERTY_HANDLE_RESOURCE_NAME 3 + +// XPropertySet +::css::uno::Reference< ::css::beans::XPropertySetInfo > OTempFileService::getPropertySetInfo() +{ + // Create a table that map names to index values. + // attention: properties need to be sorted by name! + static cppu::OPropertyArrayHelper ourPropertyInfo( + { + css::beans::Property( "Uri", PROPERTY_HANDLE_URI, cppu::UnoType<OUString>::get(), + css::beans::PropertyAttribute::READONLY ), + css::beans::Property( "RemoveFile", PROPERTY_HANDLE_REMOVE_FILE, cppu::UnoType<bool>::get(), + 0 ), + css::beans::Property( "ResourceName", PROPERTY_HANDLE_RESOURCE_NAME, cppu::UnoType<OUString>::get(), + css::beans::PropertyAttribute::READONLY ) + }, + true ); + static css::uno::Reference< css::beans::XPropertySetInfo > xInfo( + ::cppu::OPropertySetHelper::createPropertySetInfo( ourPropertyInfo ) ); + return xInfo; +} +void OTempFileService::setPropertyValue( const ::rtl::OUString& aPropertyName, const ::css::uno::Any& aValue ) +{ + if ( aPropertyName == "RemoveFile" ) + setRemoveFile( aValue.get<bool>() ); + else + { + assert(false); + throw css::beans::UnknownPropertyException(aPropertyName); + } +} +::css::uno::Any OTempFileService::getPropertyValue( const ::rtl::OUString& aPropertyName ) +{ + if ( aPropertyName == "RemoveFile" ) + return css::uno::Any(getRemoveFile()); + else if ( aPropertyName == "ResourceName" ) + return css::uno::Any(getResourceName()); + else if ( aPropertyName == "Uri" ) + return css::uno::Any(getUri()); + else + { + assert(false); + throw css::beans::UnknownPropertyException(aPropertyName); + } +} +void OTempFileService::addPropertyChangeListener( const ::rtl::OUString& /*aPropertyName*/, const ::css::uno::Reference< ::css::beans::XPropertyChangeListener >& /*xListener*/ ) +{ + assert(false); +} +void OTempFileService::removePropertyChangeListener( const ::rtl::OUString& /*aPropertyName*/, const ::css::uno::Reference< ::css::beans::XPropertyChangeListener >& /*xListener*/ ) +{ + assert(false); +} +void OTempFileService::addVetoableChangeListener( const ::rtl::OUString& /*aPropertyName*/, const ::css::uno::Reference< ::css::beans::XVetoableChangeListener >& /*xListener*/ ) +{ + assert(false); +} +void OTempFileService::removeVetoableChangeListener( const ::rtl::OUString& /*aPropertyName*/, const ::css::uno::Reference< ::css::beans::XVetoableChangeListener >& /*xListener*/ ) +{ + assert(false); +} +// XFastPropertySet +void OTempFileService::setFastPropertyValue( ::sal_Int32 nHandle, const ::css::uno::Any& aValue ) +{ + switch (nHandle) + { + case PROPERTY_HANDLE_REMOVE_FILE: setRemoveFile( aValue.get<bool>() ); return; + } + assert(false); + throw css::beans::UnknownPropertyException(OUString::number(nHandle)); +} +::css::uno::Any OTempFileService::getFastPropertyValue( ::sal_Int32 nHandle ) +{ + switch (nHandle) + { + case PROPERTY_HANDLE_REMOVE_FILE: return css::uno::Any(getRemoveFile()); + case PROPERTY_HANDLE_RESOURCE_NAME: return css::uno::Any(getResourceName()); + case PROPERTY_HANDLE_URI: return css::uno::Any(getUri()); + } + assert(false); + throw css::beans::UnknownPropertyException(OUString::number(nHandle)); +} +// XPropertyAccess +::css::uno::Sequence< ::css::beans::PropertyValue > OTempFileService::getPropertyValues() +{ + return { + css::beans::PropertyValue("Uri", PROPERTY_HANDLE_URI, css::uno::Any(getUri()), css::beans::PropertyState_DEFAULT_VALUE), + css::beans::PropertyValue("RemoveFile", PROPERTY_HANDLE_REMOVE_FILE, css::uno::Any(getRemoveFile()), css::beans::PropertyState_DEFAULT_VALUE), + css::beans::PropertyValue("ResourceName", PROPERTY_HANDLE_RESOURCE_NAME, css::uno::Any(getResourceName()), css::beans::PropertyState_DEFAULT_VALUE) + }; +} +void OTempFileService::setPropertyValues( const ::css::uno::Sequence< ::css::beans::PropertyValue >& aProps ) +{ + for ( auto const & rPropVal : aProps ) + setPropertyValue( rPropVal.Name, rPropVal.Value ); +} + namespace sdecl = ::comphelper::service_decl; sdecl::class_< OTempFileService> const OTempFileServiceImpl; const sdecl::ServiceDecl OTempFileServiceDecl( commit eb7c9bcda2a4324a682114a1f38e4761c665cbdc Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Fri Apr 5 15:37:21 2019 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Mon Apr 8 08:51:07 2019 +0200 tdf#117066 Saving ODT document with ~1500 bookmarks is slow, part 1 Individually, these don't make much difference, but they add up to a halving the time to save on my machine. EmbeddedObjectContainer shows a lot of work in HasEmbeddedObject and GetEmbeddedObjectName, so add a reverse map there. Change-Id: Ib758668dbb045e6ceb2611bd86aa2af4fbfb9917 Reviewed-on: https://gerrit.libreoffice.org/70309 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/comphelper/source/container/embeddedobjectcontainer.cxx b/comphelper/source/container/embeddedobjectcontainer.cxx index ffa22966a53b..ff0ec3a2dce5 100644 --- a/comphelper/source/container/embeddedobjectcontainer.cxx +++ b/comphelper/source/container/embeddedobjectcontainer.cxx @@ -55,19 +55,22 @@ using namespace ::com::sun::star; namespace comphelper { -typedef std::unordered_map<OUString, uno::Reference <embed::XEmbeddedObject>> -EmbeddedObjectContainerNameMap; - +typedef std::unordered_map<OUString, uno::Reference<embed::XEmbeddedObject>> EmbeddedObjectContainerNameMap; struct EmbedImpl { + struct XEmbeddedObjectRefHash + { + size_t operator()(const uno::Reference<embed::XEmbeddedObject>& rObject) const + { return reinterpret_cast<size_t>(rObject.get()); } + }; // TODO/LATER: remove objects from temp. Container storage when object is disposed - EmbeddedObjectContainerNameMap maObjectContainer; + EmbeddedObjectContainerNameMap maNameToObjectMap; + // to speed up lookup by Reference + std::unordered_map<uno::Reference<embed::XEmbeddedObject>, OUString, XEmbeddedObjectRefHash> maObjectToNameMap; uno::Reference < embed::XStorage > mxStorage; EmbeddedObjectContainer* mpTempObjectContainer; uno::Reference < embed::XStorage > mxImageStorage; uno::WeakReference < uno::XInterface > m_xModel; - //EmbeddedObjectContainerNameMap maTempObjectContainer; - //uno::Reference < embed::XStorage > mxTempStorage; bool mbOwnsStorage : 1; bool mbUserAllowsLinkUpdate : 1; @@ -197,7 +200,7 @@ EmbeddedObjectContainer::~EmbeddedObjectContainer() void EmbeddedObjectContainer::CloseEmbeddedObjects() { - for( const auto& rObj : pImpl->maObjectContainer ) + for( const auto& rObj : pImpl->maNameToObjectMap ) { uno::Reference < util::XCloseable > xClose( rObj.second, uno::UNO_QUERY ); if( xClose.is() ) @@ -229,18 +232,18 @@ OUString EmbeddedObjectContainer::CreateUniqueObjectName() uno::Sequence < OUString > EmbeddedObjectContainer::GetObjectNames() const { - return comphelper::mapKeysToSequence(pImpl->maObjectContainer); + return comphelper::mapKeysToSequence(pImpl->maNameToObjectMap); } bool EmbeddedObjectContainer::HasEmbeddedObjects() const { - return !pImpl->maObjectContainer.empty(); + return !pImpl->maNameToObjectMap.empty(); } bool EmbeddedObjectContainer::HasEmbeddedObject( const OUString& rName ) { - EmbeddedObjectContainerNameMap::iterator aIt = pImpl->maObjectContainer.find( rName ); - if (aIt != pImpl->maObjectContainer.end()) + auto aIt = pImpl->maNameToObjectMap.find( rName ); + if (aIt != pImpl->maNameToObjectMap.end()) return true; uno::Reference <container::XNameAccess> xAccess(pImpl->mxStorage, uno::UNO_QUERY); if (!xAccess.is()) @@ -250,12 +253,7 @@ bool EmbeddedObjectContainer::HasEmbeddedObject( const OUString& rName ) bool EmbeddedObjectContainer::HasEmbeddedObject( const uno::Reference < embed::XEmbeddedObject >& xObj ) const { - for( const auto& rObj : pImpl->maObjectContainer ) - { - if( rObj.second == xObj ) - return true; - } - return false; + return pImpl->maObjectToNameMap.find(xObj) != pImpl->maObjectToNameMap.end(); } bool EmbeddedObjectContainer::HasInstantiatedEmbeddedObject( const OUString& rName ) @@ -263,19 +261,19 @@ bool EmbeddedObjectContainer::HasInstantiatedEmbeddedObject( const OUString& rNa // allows to detect whether the object was already instantiated // currently the filter instantiate it on loading, so this method allows // to avoid objects pointing to the same persistence - EmbeddedObjectContainerNameMap::iterator aIt = pImpl->maObjectContainer.find( rName ); - return ( aIt != pImpl->maObjectContainer.end() ); + auto aIt = pImpl->maNameToObjectMap.find( rName ); + return ( aIt != pImpl->maNameToObjectMap.end() ); } OUString EmbeddedObjectContainer::GetEmbeddedObjectName( const css::uno::Reference < css::embed::XEmbeddedObject >& xObj ) const { - for( const auto& rObj : pImpl->maObjectContainer ) + auto it = pImpl->maObjectToNameMap.find(xObj); + if (it == pImpl->maObjectToNameMap.end()) { - if( rObj.second == xObj ) - return rObj.first; + SAL_WARN( "comphelper.container", "Unknown object!" ); + return OUString(); } - SAL_WARN( "comphelper.container", "Unknown object!" ); - return OUString(); + return it->second; } uno::Reference< embed::XEmbeddedObject> @@ -285,7 +283,7 @@ EmbeddedObjectContainer::GetEmbeddedObject( SAL_WARN_IF( rName.isEmpty(), "comphelper.container", "Empty object name!"); uno::Reference < embed::XEmbeddedObject > xObj; - EmbeddedObjectContainerNameMap::iterator aIt = pImpl->maObjectContainer.find( rName ); + auto aIt = pImpl->maNameToObjectMap.find( rName ); #if OSL_DEBUG_LEVEL > 1 uno::Reference < container::XNameAccess > xAccess( pImpl->mxStorage, uno::UNO_QUERY ); @@ -296,11 +294,11 @@ EmbeddedObjectContainer::GetEmbeddedObject( { (void)*pIter; } - OSL_ENSURE( aIt != pImpl->maObjectContainer.end() || xAccess->hasByName(rName), "Could not return object!" ); + OSL_ENSURE( aIt != pImpl->maNameToObjectMap.end() || xAccess->hasByName(rName), "Could not return object!" ); #endif // check if object was already created - if ( aIt != pImpl->maObjectContainer.end() ) + if ( aIt != pImpl->maNameToObjectMap.end() ) xObj = (*aIt).second; else xObj = Get_Impl(rName, uno::Reference<embed::XEmbeddedObject>(), pBaseURL); @@ -424,9 +422,10 @@ void EmbeddedObjectContainer::AddEmbeddedObject( const css::uno::Reference < css #endif // remember object - it needs to be in storage already - EmbeddedObjectContainerNameMap::iterator aIt = pImpl->maObjectContainer.find( rName ); - OSL_ENSURE( aIt == pImpl->maObjectContainer.end(), "Element already inserted!" ); - pImpl->maObjectContainer[ rName ] = xObj; + auto aIt = pImpl->maNameToObjectMap.find( rName ); + OSL_ENSURE( aIt == pImpl->maNameToObjectMap.end(), "Element already inserted!" ); + pImpl->maNameToObjectMap[ rName ] = xObj; + pImpl->maObjectToNameMap[ xObj ] = rName; uno::Reference < container::XChild > xChild( xObj, uno::UNO_QUERY ); if ( xChild.is() && xChild->getParent() != pImpl->m_xModel.get() ) xChild->setParent( pImpl->m_xModel.get() ); @@ -434,7 +433,7 @@ void EmbeddedObjectContainer::AddEmbeddedObject( const css::uno::Reference < css // look for object in temporary container if ( pImpl->mpTempObjectContainer ) { - auto& rObjectContainer = pImpl->mpTempObjectContainer->pImpl->maObjectContainer; + auto& rObjectContainer = pImpl->mpTempObjectContainer->pImpl->maNameToObjectMap; auto aIter = std::find_if(rObjectContainer.begin(), rObjectContainer.end(), [&xObj](const EmbeddedObjectContainerNameMap::value_type& rEntry) { return rEntry.second == xObj; }); if (aIter != rObjectContainer.end()) @@ -464,7 +463,8 @@ void EmbeddedObjectContainer::AddEmbeddedObject( const css::uno::Reference < css } // temp. container needs to forget the object - pImpl->mpTempObjectContainer->pImpl->maObjectContainer.erase( aIter ); + pImpl->mpTempObjectContainer->pImpl->maObjectToNameMap.erase( aIter->second ); + pImpl->mpTempObjectContainer->pImpl->maNameToObjectMap.erase( aIter ); } } } @@ -841,15 +841,15 @@ void EmbeddedObjectContainer::RemoveEmbeddedObject( const OUString& rName, bool bool EmbeddedObjectContainer::MoveEmbeddedObject( const OUString& rName, EmbeddedObjectContainer& rCnt ) { // find object entry - EmbeddedObjectContainerNameMap::iterator aIt2 = rCnt.pImpl->maObjectContainer.find( rName ); - OSL_ENSURE( aIt2 == rCnt.pImpl->maObjectContainer.end(), "Object does already exist in target container!" ); + auto aIt2 = rCnt.pImpl->maNameToObjectMap.find( rName ); + OSL_ENSURE( aIt2 == rCnt.pImpl->maNameToObjectMap.end(), "Object does already exist in target container!" ); - if ( aIt2 != rCnt.pImpl->maObjectContainer.end() ) + if ( aIt2 != rCnt.pImpl->maNameToObjectMap.end() ) return false; uno::Reference < embed::XEmbeddedObject > xObj; - EmbeddedObjectContainerNameMap::iterator aIt = pImpl->maObjectContainer.find( rName ); - if ( aIt != pImpl->maObjectContainer.end() ) + auto aIt = pImpl->maNameToObjectMap.find( rName ); + if ( aIt != pImpl->maNameToObjectMap.end() ) { xObj = (*aIt).second; try @@ -859,7 +859,8 @@ bool EmbeddedObjectContainer::MoveEmbeddedObject( const OUString& rName, Embedde // move object OUString aName( rName ); rCnt.InsertEmbeddedObject( xObj, aName ); - pImpl->maObjectContainer.erase( aIt ); + pImpl->maObjectToNameMap.erase( aIt->second ); + pImpl->maNameToObjectMap.erase( aIt ); uno::Reference < embed::XEmbedPersist > xPersist( xObj, uno::UNO_QUERY ); if ( xPersist.is() ) pImpl->mxStorage->removeElement( rName ); @@ -956,11 +957,12 @@ bool EmbeddedObjectContainer::RemoveEmbeddedObject( const uno::Reference < embed return false; } - auto aIter = std::find_if(pImpl->maObjectContainer.begin(), pImpl->maObjectContainer.end(), + auto aIter = std::find_if(pImpl->maNameToObjectMap.begin(), pImpl->maNameToObjectMap.end(), [&xObj](const EmbeddedObjectContainerNameMap::value_type& rEntry) { return rEntry.second == xObj; }); - if (aIter != pImpl->maObjectContainer.end()) + if (aIter != pImpl->maNameToObjectMap.end()) { - pImpl->maObjectContainer.erase( aIter ); + pImpl->maObjectToNameMap.erase( aIter->second ); + pImpl->maNameToObjectMap.erase( aIter ); uno::Reference < container::XChild > xChild( xObj, uno::UNO_QUERY ); if ( xChild.is() ) xChild->setParent( uno::Reference < uno::XInterface >() ); @@ -997,11 +999,12 @@ void EmbeddedObjectContainer::CloseEmbeddedObject( const uno::Reference < embed: { // disconnect the object from the container and close it if possible - auto aIter = std::find_if(pImpl->maObjectContainer.begin(), pImpl->maObjectContainer.end(), + auto aIter = std::find_if(pImpl->maNameToObjectMap.begin(), pImpl->maNameToObjectMap.end(), [&xObj](const EmbeddedObjectContainerNameMap::value_type& rEntry) { return rEntry.second == xObj; }); - if (aIter != pImpl->maObjectContainer.end()) + if (aIter != pImpl->maNameToObjectMap.end()) { - pImpl->maObjectContainer.erase( aIter ); + pImpl->maObjectToNameMap.erase( aIter->second ); + pImpl->maNameToObjectMap.erase( aIter ); uno::Reference < ::util::XCloseable > xClose( xObj, uno::UNO_QUERY ); try _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits