46712 - string bundle memory flushing

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P3)

Description

[Warren Harris]

From the footprint meeting 7/26/00:

9) String bundles
  Description: StringKeys used by string bundle hashtables have showed up fairly 
high on the bloatblame list. Each StringKey 
  holds an AutoString which seems wasteful for the keys in the hashtable itself 
(the AutoStrings are useful during lookup when the 
  key is stack allocated). We need to reduce the size of StringKeys. Also, could 
string bundles be arena allocated?
  Module owner: ?
  Task owner: [e-mail]
  Status: Warren to look into alternatives for StringKeys and string bundle 
allocation.


[The hashtable/nsStringKey issue is bug 46711]

Maybe string bundles can utilize arenas to save space/time.

Comment 1

[Alec Flett]

I use an arena for the string bundle cache, though not for the string bundle
itself...

In order to fix bug 46491, I needed to expose a flushBundleCache() on the string
bundle service, so that chrome could explicitly flush the string bundle cache
when the locale changes.

I'm almost done with this in my tree..

Comment 2

[Alec Flett]

Attachment: Implement flushBundles()

Comment 3

[Alec Flett]

this patch removes the dependancy on chrome, and adds the flush API.
Now going to fix chrome so that it calls this.

I'm going to reassign to me since I'm fixing two bugs at the same time - this
one and the dependancy issue.

warren/tao - care to review this so I can checkin this first patch?

Comment 4

[Alec Flett]

oops, re-add warren to CC

Comment 5

[Warren Harris]

Hiding comments from me?... :-)

Anyway, there are 2 issues here: 

1. We should remove the ConvertChromeURL stuff from string bundles. Once we do 
that, the chrome protocol will need to call your flushBundleCache routine when 
it switches locales.

2. In low-memory situations, we want to have a memory flusher registered with 
nsIMemory that calls flushBundleCache. Can you add that while you're at it? 
Thanks.

Patch looks good.

Comment 6

[Alec Flett]

Attachment: new patch - register as a memory pressure observer as well

Comment 7

[Alec Flett]

Will do. Patch attached which does that too..

Comment 8

[Warren Harris]

I don't see nsIMemoryPressureObserver in the patch.

Comment 9

[Alec Flett]

ugh, I must have attached the same patch twice

Comment 10

[Alec Flett]

ok, fix is in.. string bundles are now flushed
- when nsMemory tells us to free up memory
- when the locale switches

Comment 11

[Alec Flett]

oops, actually marking fixed

Comment 12

[Teruko Kobayashi]

Verified.