On Tue, 2013-01-08 at 14:15 +0100, Michael Stahl wrote:
> there may be more places where a similar approach could be applicable,
> both in API implementations in sw and in writerfilter/xmloff import
> code, e.g. see cc99bb9f383a65912d004e227a5b6a88b401bbba which was purely
> result of me debugging some crash and wondering why a certain insert
> method in sw core is called so often (in that case sw API impl. actually
> did the right thing).
:-) as I say, it seems to me that being able to manipulate SfxItemSet's
from UNO (or better not using UNO at all to talk to the core from the
DomainMapper ;-), might allow the same item-set to be re-used
> > It has a FIXME - I'm unclear
> > exactly what's going on there. Clearly I'm wandering at the edge of my
> > competence wrt. the writer bits here, so help/encouragement appreciated.
...
> i suppose this exception would be thrown by
> SwUnoCursorHelper::SetPropertyValues now anyway... which is actually a
> problem because in that case
> m_pImpl->m_pDoc->GetIDocumentUndoRedo().EndUndo(UNDO_INSERT, NULL);
> won't be executed and we get a confused UNDO stack in writer...
Ah right - we need the try/catch goodness there still. I've added that
back.
> i guess it's best to move the Start/EndUndo into
> SwUnoCursorHelper::SetPropertyValues to fix that (can't be omitted
> entirely since it's possible that multiple different Undo objects are
> created).
I've re-worked it as attached; I'd love to use the same code for
setting paragraph properties (which I suspect would give us another real
win there). Having said that switching the #if 1 to turn that on breaks
'make slowcheck' - though quite why is not immediately apparent to me
from code-reading ;-)
Any help getting that finished off / merged v. much appreciated.
Thanks !
Michael.
--
michael.me...@suse.com <><, Pseudo Engineer, itinerant idiot
>From e16eee537677587537b3884276a528e63a5a4c07 Mon Sep 17 00:00:00 2001
From: Michael Meeks <michael.me...@suse.com>
Date: Fri, 21 Dec 2012 15:27:27 +0000
Subject: [PATCH] fdo#44736 - set and fetch multiple properties concurrently
The domain-mapper calls SwXText::insertTextPortion very extensively
accounting for about half of import time for large, lightly formatted
text documents. The vast majority of the work is consumed managing
char + para properties - so try to batch that, making it 70% faster
for my lightly formatted test. Saves around 25% of load time for me.
Change-Id: I2582adee1bf35b07b90af810cb0d19dadc1d348f
---
sw/inc/unocrsrhelper.hxx | 13 +++++
sw/source/core/unocore/unoobj.cxx | 87 +++++++++++++++++++++++++++--------
sw/source/core/unocore/unotext.cxx | 74 ++++++++++++++++---------------
3 files changed, 118 insertions(+), 56 deletions(-)
diff --git a/sw/inc/unocrsrhelper.hxx b/sw/inc/unocrsrhelper.hxx
index a5892f6..aa7fd4d 100644
--- a/sw/inc/unocrsrhelper.hxx
+++ b/sw/inc/unocrsrhelper.hxx
@@ -151,6 +151,19 @@ namespace SwUnoCursorHelper
::com::sun::star::lang::IllegalArgumentException,
::com::sun::star::lang::WrappedTargetException,
::com::sun::star::uno::RuntimeException);
+ /// @param bTableMode: attributes should be applied to a table selection
+ void SetPropertyValues(
+ SwPaM& rPaM,
+ const SfxItemPropertySet & rPropSet,
+ const ::com::sun::star::uno::Sequence< ::com::sun::star::beans::PropertyValue > &
+ rPropertyValues,
+ const SetAttrMode nAttrMode = nsSetAttrMode::SETATTR_DEFAULT,
+ const bool bTableMode = false)
+ throw (::com::sun::star::beans::UnknownPropertyException,
+ ::com::sun::star::beans::PropertyVetoException,
+ ::com::sun::star::lang::IllegalArgumentException,
+ ::com::sun::star::lang::WrappedTargetException,
+ ::com::sun::star::uno::RuntimeException);
::com::sun::star::uno::Any GetPropertyValue(
SwPaM& rPaM,
const SfxItemPropertySet & rPropSet,
diff --git a/sw/source/core/unocore/unoobj.cxx b/sw/source/core/unocore/unoobj.cxx
index 01318bb..f5736b8 100644
--- a/sw/source/core/unocore/unoobj.cxx
+++ b/sw/source/core/unocore/unoobj.cxx
@@ -1861,34 +1861,81 @@ throw (beans::UnknownPropertyException, beans::PropertyVetoException,
lang::IllegalArgumentException, lang::WrappedTargetException,
uno::RuntimeException)
{
+ uno::Sequence< beans::PropertyValue > aValues(1);
+ aValues[0].Name = rPropertyName;
+ aValues[0].Value = rValue;
+ SetPropertyValues(rPaM, rPropSet, aValues, nAttrMode, bTableMode);
+}
+
+void SwUnoCursorHelper::SetPropertyValues(
+ SwPaM& rPaM, const SfxItemPropertySet& rPropSet,
+ const uno::Sequence< beans::PropertyValue > &rPropertyValues,
+ const SetAttrMode nAttrMode, const bool bTableMode)
+throw (beans::UnknownPropertyException, beans::PropertyVetoException,
+ lang::IllegalArgumentException, lang::WrappedTargetException,
+ uno::RuntimeException)
+{
+ if (!rPropertyValues.getLength())
+ return;
+
SwDoc *const pDoc = rPaM.GetDoc();
- SfxItemPropertySimpleEntry const*const pEntry =
- rPropSet.getPropertyMap().getByName(rPropertyName);
- if (!pEntry)
+ rtl::OUString aUnknownExMsg, aPropertyVetoExMsg;
+
+ // Build set of attributes we want to fetch
+ std::vector<sal_uInt16> aWhichPairs;
+ std::vector<SfxItemPropertySimpleEntry const*> aEntries;
+ aEntries.reserve(rPropertyValues.getLength());
+ for (sal_Int32 i = 0; i < rPropertyValues.getLength(); ++i)
{
- throw beans::UnknownPropertyException(
- OUString(RTL_CONSTASCII_USTRINGPARAM("Unknown property: "))
- + rPropertyName,
- static_cast<cppu::OWeakObject *>(0));
+ const rtl::OUString &rPropertyName = rPropertyValues[i].Name;
+
+ SfxItemPropertySimpleEntry const* pEntry =
+ rPropSet.getPropertyMap().getByName(rPropertyName);
+
+ // Queue up any exceptions until the end ...
+ if (!pEntry)
+ {
+ aUnknownExMsg += "Unknown property: '" + rPropertyName + "' ";
+ break;
+ }
+ else if (pEntry->nFlags & beans::PropertyAttribute::READONLY)
+ {
+ aPropertyVetoExMsg += "Property is read-only: '" + rPropertyName + "' ";
+ break;
+ } else {
+// FIXME: we should have some nice way of merging ranges surely ?
+ aWhichPairs.push_back(pEntry->nWID);
+ aWhichPairs.push_back(pEntry->nWID);
+ }
+ aEntries.push_back(pEntry);
}
- if (pEntry->nFlags & beans::PropertyAttribute::READONLY)
+ if (!aWhichPairs.empty())
{
- throw beans::PropertyVetoException(
- OUString(RTL_CONSTASCII_USTRINGPARAM("Property is read-only: "))
- + rPropertyName,
- static_cast<cppu::OWeakObject *>(0));
- }
+ aWhichPairs.push_back(0); // terminate
+ SfxItemSet aItemSet(pDoc->GetAttrPool(), &aWhichPairs[0]);
- SfxItemSet aItemSet( pDoc->GetAttrPool(), pEntry->nWID, pEntry->nWID );
- SwUnoCursorHelper::GetCrsrAttr( rPaM, aItemSet );
+ // Fetch, overwrite, and re-set the attributes from the core
+ SwUnoCursorHelper::GetCrsrAttr( rPaM, aItemSet );
- if (!SwUnoCursorHelper::SetCursorPropertyValue(
- *pEntry, rValue, rPaM, aItemSet))
- {
- rPropSet.setPropertyValue(*pEntry, rValue, aItemSet );
+ for (sal_Int32 i = 0; ( i < rPropertyValues.getLength() &&
+ i < (sal_Int32)aEntries.size() ); ++i)
+ {
+ const uno::Any &rValue = rPropertyValues[i].Value;
+ SfxItemPropertySimpleEntry const*const pEntry = aEntries[i];
+ if (!pEntry)
+ continue;
+ if (!SwUnoCursorHelper::SetCursorPropertyValue(*pEntry, rValue, rPaM, aItemSet))
+ rPropSet.setPropertyValue(*pEntry, rValue, aItemSet);
+ }
+
+ SwUnoCursorHelper::SetCrsrAttr(rPaM, aItemSet, nAttrMode, bTableMode);
}
- SwUnoCursorHelper::SetCrsrAttr(rPaM, aItemSet, nAttrMode, bTableMode);
+
+ if (!aUnknownExMsg.isEmpty())
+ throw beans::UnknownPropertyException(aUnknownExMsg, static_cast<cppu::OWeakObject *>(0));
+ if (!aPropertyVetoExMsg.isEmpty())
+ throw beans::PropertyVetoException(aPropertyVetoExMsg, static_cast<cppu::OWeakObject *>(0));
}
uno::Sequence< beans::PropertyState >
diff --git a/sw/source/core/unocore/unotext.cxx b/sw/source/core/unocore/unotext.cxx
index d2e3add..8060a1b 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -17,6 +17,7 @@
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
*/
+#include <stdio.h>
#include <stdlib.h>
@@ -1309,6 +1310,7 @@ throw (lang::IllegalArgumentException, uno::RuntimeException)
{
aPam.Move( fnMoveBackward, fnGoNode );
}
+#if 1
if (rProperties.getLength())
{
// now set the properties
@@ -1345,6 +1347,26 @@ throw (lang::IllegalArgumentException, uno::RuntimeException)
}
}
}
+#else
+ try
+ {
+ SfxItemPropertySet const*const pParaPropSet =
+ aSwMapProvider.GetPropertySet(PROPERTY_MAP_PARAGRAPH);
+ if (!bIllegalException)
+ SwUnoCursorHelper::SetPropertyValues(aPam, *pParaPropSet, rProperties);
+ }
+ catch (const lang::IllegalArgumentException& rIllegal)
+ {
+ sMessage = rIllegal.Message;
+ bIllegalException = true;
+ }
+ catch (const uno::RuntimeException& rRuntime)
+ {
+ sMessage = rRuntime.Message;
+ bRuntimeException = true;
+ }
+#endif
+
m_pDoc->GetIDocumentUndoRedo().EndUndo(UNDO_END, NULL);
if (bIllegalException || bRuntimeException)
{
@@ -1415,43 +1437,23 @@ throw (lang::IllegalArgumentException, uno::RuntimeException)
pCursor->GetPoint()->nContent = nContentPos;
}
- if (rCharacterAndParagraphProperties.getLength())
+ try
{
- const SfxItemPropertyMap &rCursorMap =
- aSwMapProvider.GetPropertySet(PROPERTY_MAP_TEXT_CURSOR)
- ->getPropertyMap();
- beans::PropertyValue const*const pValues =
- rCharacterAndParagraphProperties.getConstArray();
- SfxItemPropertySet const*const pCursorPropSet =
- aSwMapProvider.GetPropertySet(PROPERTY_MAP_TEXT_CURSOR);
- const sal_Int32 nLen(rCharacterAndParagraphProperties.getLength());
- for (sal_Int32 nProp = 0; nProp < nLen; ++nProp)
- {
- if (!rCursorMap.getByName( pValues[nProp].Name ))
- {
- bIllegalException = true;
- break;
- }
- try
- {
- SwUnoCursorHelper::SetPropertyValue(
- *pCursor, *pCursorPropSet,
- pValues[nProp].Name, pValues[nProp].Value,
- nsSetAttrMode::SETATTR_NOFORMATATTR);
- }
- catch (const lang::IllegalArgumentException& rIllegal)
- {
- sMessage = rIllegal.Message;
- bIllegalException = true;
- break;
- }
- catch (const uno::RuntimeException& rRuntime)
- {
- sMessage = rRuntime.Message;
- bRuntimeException = true;
- break;
- }
- }
+ SfxItemPropertySet const*const pCursorPropSet =
+ aSwMapProvider.GetPropertySet(PROPERTY_MAP_TEXT_CURSOR);
+ SwUnoCursorHelper::SetPropertyValues(*pCursor, *pCursorPropSet,
+ rCharacterAndParagraphProperties,
+ nsSetAttrMode::SETATTR_NOFORMATATTR);
+ }
+ catch (const lang::IllegalArgumentException& rIllegal)
+ {
+ sMessage = rIllegal.Message;
+ bIllegalException = true;
+ }
+ catch (const uno::RuntimeException& rRuntime)
+ {
+ sMessage = rRuntime.Message;
+ bRuntimeException = true;
}
m_pImpl->m_pDoc->GetIDocumentUndoRedo().EndUndo(UNDO_INSERT, NULL);
if (bIllegalException || bRuntimeException)
--
1.7.7
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
