403589 - Fuse memory allocations in gfxTextRun

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

gfxTextRun::gfxTextRun allocates an mCharacterGlyphs array and a string. These allocations could be fused into a single allocation which would be faster, reduce memory usage and fragmentation, and improve locality. Stuart says this is worth doing.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in.

Comment 3

[Fabien Tassin]

it's still failing for me:

g++-4.2 -o gfxXlibNativeRenderer.o -c -I../../../dist/include/system_wrappers -include ../../../config/gcc_hidden.h -DIMPL_THEBES -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_COM_OBSOLETE -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux  -I../../../dist/include/cairo -I../../../dist/include/libpixman -I../../../dist/include/string -I../../../dist/include/pref -I../../../dist/include/xpcom -I../../../dist/include/unicharutil -I../../../dist/include/lcms -I../../../dist/include   -I../../../dist/include/thebes -I/usr/include/nspr -I/usr/include  -I/usr/include  -I../../../dist/sdk/include    -fPIC   -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pedantic -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -pipe -w -O2 -fno-strict-aliasing -g -I../../../dist/include/cairo -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12   -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12     -DMOZILLA_CLIENT -include ../../../mozilla-config.h -Wp,-MD,.deps/gfxXlibNativeRenderer.pp gfxXlibNativeRenderer.cpp
gfxPangoFonts.cpp
g++-4.2 -o gfxPangoFonts.o -c -I../../../dist/include/system_wrappers -include ../../../config/gcc_hidden.h -DIMPL_THEBES -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_COM_OBSOLETE -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux  -I../../../dist/include/cairo -I../../../dist/include/libpixman -I../../../dist/include/string -I../../../dist/include/pref -I../../../dist/include/xpcom -I../../../dist/include/unicharutil -I../../../dist/include/lcms -I../../../dist/include   -I../../../dist/include/thebes -I/usr/include/nspr -I/usr/include  -I/usr/include  -I../../../dist/sdk/include    -fPIC   -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pedantic -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -pipe -w -O2 -fno-strict-aliasing -g -I../../../dist/include/cairo -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12   -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12     -DMOZILLA_CLIENT -include ../../../mozilla-config.h -Wp,-MD,.deps/gfxPangoFonts.pp gfxPangoFonts.cpp
gfxPangoFonts.cpp: In member function 'virtual gfxTextRun* gfxPangoFontGroup::MakeTextRun(const PRUint8*, PRUint32, const gfxTextRunFactory::Parameters*, PRUint32)':
gfxPangoFonts.cpp:577: error: no matching function for call to 'gfxTextRun::operator new(unsigned int)'
../../../dist/include/thebes/gfxFont.h:1257: note: candidates are: static void* gfxTextRun::operator new(size_t, PRUint32, PRUint32)
gfxPangoFonts.cpp: In member function 'virtual gfxTextRun* gfxPangoFontGroup::MakeTextRun(const PRUnichar*, PRUint32, const gfxTextRunFactory::Parameters*, PRUint32)':
gfxPangoFonts.cpp:618: error: no matching function for call to 'gfxTextRun::operator new(unsigned int)'
../../../dist/include/thebes/gfxFont.h:1257: note: candidates are: static void* gfxTextRun::operator new(size_t, PRUint32, PRUint32)
make[6]: *** [gfxPangoFonts.o] Error 1

shouldn't you also fix gfxFont.h ?

Comment 4

[Peter Weilbacher]

Fabien, are you sure you updated your source tree after the checkin? Looking at mine I see that the "new gfxTextRun" calls have gone from gfxPangoFonts...

Comment 5

[Fabien Tassin]

Peter,
I fetched fresh sources (xulrunner) 2 hours ago (and got 2007-11-16 01:51) and I did a full build from scratch, no re-use of anything.
According to bonsai, you committed your fix at 2007-11-15 17:52.
So i'm puzzled.

Comment 6

[Peter Weilbacher]

It was actually Robert who committed the fix at 2007-11-15 17:52. Please check in the CVS/Entries file under gfx/thebes/src if you really got version 1.116 of gfxPangoFonts.cpp. If not (or even if yes) try to do a cvs up -PAC gfx/thebes/src/gfxPangoFonts.cpp and try to rebuild just in gfx/thebes.

Comment 7

[Fabien Tassin]

oops, sorry for the mistake of who committed what.

I've re-triggered my bot, it detected new sources, got 2007-11-16 12:50, and it built fine. Sorry for the troubles.