97172 - startup perf- remove the need of loading of maccharset.properties files at startup time to speed up

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Frank Tang]

we currently load maccharset.properties at startup time, we should find a way to 
remove the need of loading it at startup time to speed up startup performance
we could 
1. build in common used pair in the cpp code, and/or
2. cache the resolved result in registry.

Comment 1

[[e-mail] (away - not reading bugmail)]

per offline discussion with ftang - tentatively reassigning to myself

Comment 2

[Frank Tang]

mac sutff, move to ftang for now.

Comment 3

[Frank Tang]

nhotta- can you handle this?

what we should do is
1. move the loading of property file from constructor to a seperate 
InitInfo() function
2. build in some common used mapping into c++ (for US, JA, DE and FR ) so we 
don't need to look at those mapping in the property file if we running on those 
tier 1 platform
3. call InitInfo right before we really need to access those mapping.
In this way, we keep both the dynamic part and tuning the performance at the 
same time. 

Comment 4

[nhottanscp]

Attachment: Delay the loading of the .property file, also avoid to load the .property file if it's found in the hard coded strings.

Comment 5

[nhottanscp]

Attachment: Forgot to pass an argument as a reference for the new function.

Comment 6

[Frank Tang]

-  PR_AtomicIncrement(&gCnt);
why you want to do that ? we should always have balance of

110   PR_AtomicDecrement(&gCnt); 
in the destructory. if you remove this you proably also need to change the 
destrcutor

+    PR_AtomicIncrement(&gCnt);
gCnt is the count of nsMacCharset object. it should be in the contstructor.

+    if (NS_FAILED(rv)) { charset.AssignWithConversion("x-mac-roman");}
please break the if statement into multiple line
don't put the body of the block and the if in the same line.
we won't be able to set break point in the body of the block

Comment 7

[nhottanscp]

Attachment: Patch, updated to ftang's comment.

Comment 8

[Frank Tang]

Comment on attachment 52649 [details] [diff] [review]
Patch, updated to ftang's comment.

look fine r=ftang

Comment 9

[Simon Fraser [no longer active]]

+    nsAutoString propertyURL(NS_LITERAL_STRING("resource:/res/
maccharset.properties"));
+    nsURLProperties *info = new nsURLProperties( propertyURL );

Not your fault, but nsURLProperties() needs to be updated to use some newer 
string stuff (e.g. is its argument read-only?).

+nsresult nsMacCharset::MapToCharset(short script, short region, nsString& 
charset)
Is 'charset' an out param? Rename it to make this clearer (e.g. 'outCharset').

+    rv = pMacLocale->GetPlatformLocale(&localeAsString, &script, &language, &
region);
Ugh, GetPlatformLocale takes a nsString*. Needs fixing later.

Rename 'charset' and sr=sfraser.

Comment 10

[nhottanscp]

Attachment: Patch, name change, 'charset' to 'outCharset' for a function argument.

Comment 11

[nhottanscp]

Checked in to the trunk.

Comment 12

[Teruko Kobayashi]

Changed QA contact to [e-mail].