[PUSHED 3-6] Fix for fdo#53929 - Case insensitive strings in pivot table

Michael Meeks michael.meeks at suse.com
Thu Aug 23 01:13:44 PDT 2012


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 at suse.com  <><, Pseudo Engineer, itinerant idiot



More information about the LibreOffice mailing list