Gfx:Gtk (including Xft) and Gfx:Xlib need a clean-up observer to free global variables

RESOLVED WORKSFORME

Status

Core Graveyard
GFX: Gtk
RESOLVED WORKSFORME
14 years ago
9 years ago

People

(Reporter: Jungshik Shin, Assigned: Jungshik Shin)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

14 years ago
Currently, dtors of nsFontMetricsGTK/Xlib/Xft call FreeGlobal() that clean up
static global variables. This is certainly not efficient because nsFontMetrics*
are created and destructed many times during the life time of Mozilla, but
static global variables need not be released and recreated everytime this happens. 

nsFontMetricsWin uses nsIObserver and calls FreeGlobal only once when Mozilla is
being shut down.
(Assignee)

Comment 1

14 years ago
sorry for spamming. forgot to assign to myself.
Assignee: blizzard → jshin
(Assignee)

Comment 2

14 years ago
Hmm. it's a bit embarrasing. I forgot that the number of nsFontMetrics(GTK|Xft)
instances is kept and FreeGlobal() is called only when the instance counter goes
to zero. Therefore, we don't have to bother to add a clean-up observer
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → INVALID

Comment 3

14 years ago
And print'ing that counter never gives zero? [there was something similar in
GfxWin, but shanjian said that he saw the counter hitting zero and so he
switched to the observer.]
(Assignee)

Comment 4

14 years ago
I was worried a little bit that that might happen, but dashed it out because it
looks like it should not happen (in theory). Now that you told me that had
happened in nsFontMetricsWin, I have to reconsider. Perhaps, as you suggested, I
have to add a debug statement, browse for a while (perhaps quite a long while)
and see what happens. 
Status: RESOLVED → REOPENED
Resolution: INVALID → ---

Comment 5

14 years ago
jshin:
At least for gfx/src/xlib the problem described in comment #0 was never observed
so I guess it is not present... and since the gfx/src/xlib code is more or less
a GDK/GTK+-less clone of gfx/src/gtk the same conclusion should apply to
gfx/src/gtk, too...

Comment 6

14 years ago
Quick reference: the GfxWin counter was removed in bug 74899.
(Assignee)

Comment 7

14 years ago
rbs, thanks for the reference.
Roland, thanks for the info. I'm just looking at Xlib code and realized what I
forgot : Xlib code is structrually a bit different from Gtk code because the
former uses nsFontMetricsXlibContext. BTW, is this taken care of? 

1121   // XXX many of these statics need to be freed at shutdown time
    http://lxr.mozilla.org/seamonkey/source/gfx/src/xlib/nsFontMetricsXlib.cpp#1121
(Assignee)

Comment 8

14 years ago
The instance counter never went zero in my 30 minute session with an Xft build.
It seems like we're fine as it is now. (comment #5). Anyway, I'll try
Gtk(X11core) just in case.

Comment 9

14 years ago
Created attachment 151085 [details]
Sample shows inconsistence of variable value on button event "onClick"
gfx/src/xlib and gfx/src/gtk have been removed on trunk.
Please file a new bug if this still occurs in a recent trunk build. Thanks.
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

-> WORKSFORME
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago11 years ago
Resolution: --- → WORKSFORME
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.