neil@parkwaycc.co.uk Reporter
	Description • 22 years ago

Wondering why it takes Mail 3-4 seconds to sort 12500 strings, I ran jprof and
found that more than half of this time is used within GetCharsetAtom! I
converted it to using an instance reference to the charset alias service, and
jprof now says GetCharsetAtom takes 2/3 of the time and the sort is down to 2-3s.

	neil@parkwaycc.co.uk Reporter
	Comment 2 • 22 years ago

Actually I think there's further room for improvement creating collation keys.

	neil@parkwaycc.co.uk Reporter
	Comment 3 • 22 years ago

I just got a better idea.

	neil@parkwaycc.co.uk Reporter
	Comment 6 • 22 years ago

Comment on attachment 120568 [details] [diff] [review]
Proposed patch

This patch makes sorting messages 200% faster on Linux and probably most other
OSes e.g. Win9x but NOT WinNT which already uses fast native unicode sorting.

	Simon Montagu :smontagu Assignee
	Comment 7 • 22 years ago

This looks good, but could we go further? Could GetDefaultCharsetForLocale
return a charset atom instead of a nsString? (If it has to be a string I would
prefer an nsCString, but that's another story.)

	Alec Flett
	Comment 8 • 22 years ago

Comment on attachment 120569 [details]
diff -w so you can read it

hmm. I have to agree, why don't we just cache the atom?

	neil@parkwaycc.co.uk Reporter
	Comment 9 • 22 years ago

OK but I didn't want to go mucking around with the interfaces without approval.

	Alec Flett
	Comment 10 • 22 years ago

approval from whom? :) as long as the interface isn't marked FROZEN (and maybe
UNDER_REVIEW) then the interface is just like any other code - you get the right
module owner review and/or approval and so forth..

	neil@parkwaycc.co.uk Reporter
	Comment 11 • 22 years ago

OK, so have I got this right?
The only user of nsCollation is nsCollation<platform>.
These never change mCharset after they create nsCollation.
So, they can pass mCharset to a nsCollation method to create the converter.
In fact, I pass the PRUnichar * to save some conversions.
If you forget or fail to set the charset then it falls back on ISO-8859-1.

	neil@parkwaycc.co.uk Reporter
	Comment 12 • 22 years ago

Comment on attachment 120821 [details] [diff] [review]
New approach

OS/2 never does unicode conversion so I just junked all the charset code.

	neil@parkwaycc.co.uk Reporter
	Comment 13 • 22 years ago

Oh, and should this get r+sr feel free to check it in before I can on Tuesday.

	Alec Flett
	Comment 14 • 22 years ago

Comment on attachment 120821 [details] [diff] [review]
New approach

nice. this is a million times better. Sorry for the delay in reading it...

sr=alecf

smontagu, can you r= this  in time for 1.4 beta?

	Simon Montagu :smontagu Assignee
	Comment 15 • 22 years ago

Comment on attachment 120821 [details] [diff] [review]
New approach

r=smontagu

	Simon Montagu :smontagu Assignee
	Comment 16 • 22 years ago

cc-ing mkaply in case there are issues with comment 12.

	Mike Kaply [:mkaply]
	Comment 17 • 22 years ago

Neil and I talked about this and we think we are OK.

	Simon Montagu :smontagu Assignee
	Comment 18 • 22 years ago

Fix checked in.

	Jan Varga [:janv]
	Comment 19 • 22 years ago

good work, this may help bug 160140