Date: prev next · Thread: first prev next last
2012 Archives by date, by thread · List index


Hi Kohei,

On Wed, 2012-08-22 at 23:39 -0400, Kohei Yoshida wrote:
Another fix I'd like backported to 3-6 (and perhaps 3-6-1 if enough 
people feel comfortable) is this

        I love the unit test too :-)

https://gerrit.libreoffice.org/gitweb?p=core.git;a=commitdiff;h=1afd1e5ca8872253c491af76c70397fb9e00f900

        Having said that - whenever I see the end of:

@@ -210,8 +210,9 @@ bool ScDPItemData::IsCaseInsEqual(const
ScDPItemData& r) const
;
}

 if (mbStringInterned && r.mbStringInterned && mpString == r.mpString)
     // Fast equality check for interned strings.
     return true;

  return ScGlobal::GetpTransliteration()->isEqual(GetString(), r.GetString());
}

        I hear a very heavy grinding sound as UNO infrastructure picks up
steam, and the combination with the i18n heavy-lifting there makes me
hear performance screaming from the users ringing in my ears [ I must
get some tablets for that ;-].

        As such, throwing babies overboard to avoid calling isEqual() seems
like a good plan to me ;-) for a start - I'm interested in why we only
do a string equality comparison if these mbStringInterned things are
set; surely it's smaller and easier to just do the pointer compare:

        if (mpString == r.mpString)

        is just as good presumably ? except - with some more thinking:

        Also, I wonder how often the data-pilot code will do this ? presumably
if we build a hash-table of strings we know are not equal (I guess the
DP has bucketing code for this already), it'd be far faster to look both
strings up in that bucket, detect they are not the same and then
exit ;-) [ assuming the bucket is constructed case-insensitively ].

        Perhaps this was the function of the pre-existing check ? - if both
strings are case-sensitively interned in a hash, we be sure that they
are not equal if they are both in there, and the pointers don't match -
without the nightmare weight lifting ? :-)

        Anyhow - I've pushed it to -3-6 since it clearly helps.

        Thoughts ?

                Michael.

-- 
michael.meeks@suse.com  <><, Pseudo Engineer, itinerant idiot


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.