If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

string bundle memory flushing

VERIFIED FIXED

Status

()

Core
Internationalization
P3
normal
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Warren Harris, Assigned: Alec Flett)

Tracking

({footprint})

Trunk
All
Other
footprint
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3+])

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
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: warren@netscape.com
  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.
(Reporter)

Updated

17 years ago
Keywords: footprint, nsbeta3
(Reporter)

Updated

17 years ago
Whiteboard: [nsbeta3+]
(Assignee)

Comment 1

17 years ago
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..

(Assignee)

Comment 2

17 years ago
Created attachment 12350 [details] [diff] [review]
Implement flushBundles()
(Assignee)

Comment 3

17 years ago
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?
Assignee: warren → alecf
(Assignee)

Comment 4

17 years ago
oops, re-add warren to CC
Status: NEW → ASSIGNED
(Reporter)

Comment 5

17 years ago
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.
(Assignee)

Comment 6

17 years ago
Created attachment 12353 [details] [diff] [review]
new patch - register as a memory pressure observer as well
(Assignee)

Comment 7

17 years ago
Will do. Patch attached which does that too..
(Reporter)

Comment 8

17 years ago
I don't see nsIMemoryPressureObserver in the patch.
(Assignee)

Comment 9

17 years ago
ugh, I must have attached the same patch twice
(Assignee)

Comment 10

17 years ago
ok, fix is in.. string bundles are now flushed
- when nsMemory tells us to free up memory
- when the locale switches
(Assignee)

Comment 11

17 years ago
oops, actually marking fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 12

17 years ago
Verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.