Issue 100177 - performance: impl_ts_getLocale result should be reused
Summary: performance: impl_ts_getLocale result should be reused
Status: CLOSED FIXED
Alias: None
Product: General
Classification: Code
Component: code (show other issues)
Version: OOO310m5
Hardware: All All
: P3 Trivial (vote)
Target Milestone: OOo 3.1
Assignee: stefan.baltzer
QA Contact: issues@framework
URL:
Keywords: aqua, performance, regression
Depends on:
Blocks: 98052 95768
  Show dependency tree
 
Reported: 2009-03-13 13:22 UTC by hdu@apache.org
Modified: 2009-05-08 10:04 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description hdu@apache.org 2009-03-13 13:22:19 UTC 
While debugging into issue 98052 and issue 100172 it showed that the method impl_ts_getLocale() is 
reponsible for almost half of the multi-second wait because the underlying configmgr is so slow. The 
header file comments that
> We does not cache this value, because we are not listen for changes on the configuration layer ...
but it is easily possible to reduce the 140 expensive invokations to 4 without loosing anything by 
moving the expensive call outside the inner loop in impl_ts_load(). On this dualcore 2GHz system the 
patch below saves more than a second:

--- a/framework/source/accelerators/acceleratorconfiguration.cxx
+++ b/framework/source/accelerators/acceleratorconfiguration.cxx
@@ -1247,2 +1245,3 @@
         css::uno::Sequence< ::rtl::OUString > lKeys = xAccess->getElementNames();
+        const ::rtl::OUString sLocale = impl_ts_getLocale().toISO();
         sal_Int32 nKeys = lKeys.getLength();
@@ -1264,1 +1262,1 @@
-                if ( *pFound == impl_ts_getLocale().toISO() )
+                if ( *pFound == sLocale )
Comment 1 hdu@apache.org 2009-03-13 14:31:53 UTC 
s/sLocale/sCfgLocale/
Comment 2 eric.bachard 2009-03-14 14:06:19 UTC 
+me on CC

@hdu : I have tested this patch (with DEV300_m43) + the other patch found at issue 100172 : the change 
is really visible. thanks a lot !!
Comment 3 eric.bachard 2009-03-14 14:06:37 UTC 
.
Comment 4 hdu@apache.org 2009-03-16 08:59:49 UTC 
@ericb: thanks for testing the fix. The problem really proves that one should never trust explanations for 
performance issues unless they have been analyzed. The two performance bugs mentioned were 
responsible for much more of the UI-delay than the real work (loading the spell-checker, hyphenator, 
grammar checker and keyboard shortcuts).
Comment 5 carsten.driesner 2009-03-16 09:19:23 UTC 
cd->hdu: We discussed by phone with as that moving the line out of the loop is
an easy fix with is NOT dangerous.

cd: Therefore approved by me and as.
Comment 6 carsten.driesner 2009-03-16 09:20:28 UTC 
cd: Add regression keyword. Ok in OOo 3.0.1.
Comment 7 carsten.driesner 2009-03-16 10:52:28 UTC 
cd: Started.
Comment 8 carsten.driesner 2009-03-16 13:00:05 UTC 
cd: Set correct target. Approved as show stopper.
Comment 9 hdu@apache.org 2009-03-16 14:42:09 UTC 
code change reviewed => ok

Copying the locales into the vector seems like a waste of time too. Compared to the improvements by the 
current fix it is only a minor issue though.
Comment 10 carsten.driesner 2009-03-16 17:20:50 UTC 
cd: Fixed.
Comment 11 carsten.driesner 2009-03-16 17:21:49 UTC 
cd->sba: Please verify.
Comment 12 stefan.baltzer 2009-03-20 17:47:23 UTC 
Verified in CWS fwk106.
Comment 13 stefan.baltzer 2009-05-08 10:04:55 UTC 
OK in OOO310_m11 (OOo 3.1). Closed.