Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Description • 18 years ago

Now, we have a ts regression, the cause is bug 362665.

The patch of bug 362665 creates "perfect" font list at starting up.
If we delay the some part of the process, we may fix this regression.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 1 • 18 years ago

Note that I will attach a proposal patch, but how to test it?

And I don't want to back out the current patch. Because pre bug 362665 builds are not usable build for CJK users. (see bug 357555) The perf regression is not a critical, but bug 357555 is critical for CJK testers.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 2 • 18 years ago

Bug 364678 checked-in creates more regression.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 3 • 18 years ago

This patch is a minimum patch, and I don't know whether it's fast how.

1. FontEntry doesn't initialize at creating, it will be initialized at first using.
2. |ATSUGetIndFontName| is called only once per a font name.(current trunk calls it twice per a font name.)
3. The text converter is not created every time. If the encoding is changed, the converter will be recreated.

I have another idea, but I need more time for the another patch.(And I don't know whether the another approach will work fine.)

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 4 • 18 years ago

Ts testing on my environment.

current build:
2733, 2144, 2126, 2538, 2229, 2261, 2378, 2124, 2694, 2149

patched build
2593, 2176, 2457, 2132, 2139, 2159, 2046, 2074, 2124, 2146

AVG 2337.6 vs 2204.6
But the values of current build are shaken. I don't know whether we can trust this test.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 5 • 18 years ago

Thank you vald. I checked-in the patch.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 6 • 18 years ago

MacOSX Darwin 8.8.4 bm-xserve08 Dep
984 -> 951 -> 953

MacOSX Darwin 8.7.2 xserve06 Dep Universal Nightly
1918 -> 1840 -> 1833

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 8 • 18 years ago

This is a final patch. I don't have more idea for improve the performance.

This patch only caches the mac font names. Other names will be cached in ResolveFontName at first using. I want to cache the one name only per font. But the mac font name cannot be found by ATSUFindFontFromName with Unicode name. I think that if we want to find by the API, we need to convert the given name to correct encoding. But we cannot know the encoding from given name. Therefore, I cannot create the "best" patch...

But this patch gets the postscript name at first, by this, the redundant loops can be removed from the end of the for loop for |fontIDs|.

And I remove the nsAutoString variables from for loops.

And this patch also fixes bug 364832.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 9 • 18 years ago

Removing the redundant code.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 10 • 18 years ago

Comment on attachment 249787 [details] [diff] [review]
Patch #2 rv1.1

This patch has a bug.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 11 • 18 years ago

We need one more step. Because [NSFont fontName] sometimes returns a known font that cannot be listed up by ATSUI. We should resolve the basic family name to postscript name at first using.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 12 • 18 years ago

(In reply to comment #11)
> We need one more step. Because [NSFont fontName] sometimes returns a known font

-> Because [NSFont fontName] sometimes returns an *unknown* font

sorry.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 13 • 18 years ago

checked-in.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 14 • 18 years ago

I forgot a comment.

I think that this patch will be faster than current trunk build. But it is slower than pre bug 362665. Because there are necessary code -so, we cannot remove it- in starting up code. If you can write faster code, please file a new bug. This patch has a best performance I can think now.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 15 • 18 years ago

Umm... The patch is not faster than the previous build (but not slower) in tinderbox :-(

Irrespective of the amount the listed up font names is decreased...

This bug only wins by patch rv1.0 (see comment 6).
# The patch #2 rv2.0 fixes other bug too, we don't need to back out it.

	Frank Wein [:mcsmurf]
	Comment 16 • 16 years ago

Changeset d292ca6cf27b in mozilla-central referenced this bug here. The bug # in the check-in comment was wrong, correct bug # is Bug 480698.