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

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P3)

Description

[Frank Tang]

we currently load wincharset.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

[Frank Tang]

reassign to jbetak to work on. mark m0.9.6

Comment 2

[Frank Tang]

jbetak said we can do it in m0.9.6

Comment 3

[kill this account]

Juraj: would you kindly tell us how much time will be saved by doing this?

Comment 4

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

accepting. 

bstell:

my measurements using the built-in NS_TIMELINE timer show that this would save 
~31 ms, which ~0.6% of my local startup time. By doing this with say 10 files, 
we could potentially save 6% of startup time and get an honorable mention in 
the next release.

Comment 5

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

patch is nearly ready - I'm borrowing from the changes nhotta made to
nsMacCharset.cpp

Comment 6

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

Attachment: patch leaning on attchment 52844 by nhotta

Comment 7

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

nhotta or ftang - could you please help with a review. I just quickly put this 
together, so there might be things that I missed. Thanks!

Comment 8

[nhottanscp]

The changes for nsCharsetMenu.cpp, nsCharsetAliasImp.cpp, are those all needed?

Comment 9

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

Attachment: removed overlap with another patch - thanks nhotta!

Comment 10

[nhottanscp]

Comment on attachment 54319 [details] [diff] [review]
removed overlap with another patch - thanks nhotta!

Please check the result of InitInfo() in MapToCharset().
r=nhotta

Comment 11

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

Attachment: revised patch - thanks nhotta!

Comment 12

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

sfraser:

this is analogous to bug 97172 which pertains to maccharset.properties and I 
was wondering whether you'd feel comfortable with super reviewing this patch as 
well.

Comment 13

[Simon Fraser [no longer active]]

Attachment 54511 [details] [diff] does not look like a patch to me.

Comment 14

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

Attachment: oops - I apologize, this should be the most recent revision!

Comment 15

[Simon Fraser [no longer active]]

Comment on attachment 54516 [details] [diff] [review]
oops - I apologize, this should be the most recent revision!

> private:
>   nsString mCharset;
>+  nsresult InitInfo();
>+  nsresult MapToCharset(nsString& inANSICodePage, nsString& outCharset);

Minor stylistic comment; I don't like to see private members and methods mixed
up like this. It seems cleaner to have:

private:

  nsresult InitInfo();
  nsresult MapToCharset(nsString& inANSICodePage, nsString& outCharset);

private:

  nsString mCharset;

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

Can you use an NS_LITERAL_STRING here?

>+  if(inANSICodePage.Equals(NS_LITERAL_STRING("acp.1252"))) {

Non-standard spacing ("if (").

>+	nsString					charset;
> 	nsString					localeAsNSString(localeName);

Would nsAutoStrings be better?

Fix those and sr=sfraser

Comment 16

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

Attachment: thanks sfraser!

Comment 17

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

fix checked in - thanks everyone!

Comment 18

[Teruko Kobayashi]

Changed QA contact to [e-mail].