GNOME Bugzilla – Bug 168899
Add const to reduce non-shared data
Last modified: 2005-03-10 15:18:47 UTC
It is read-only after all... The 'const char *name' could probably also be changed to 'const char name[23]'
Created attachment 38106 [details] [review] proposed patch
break.c seems to have a pile of arrays that might become const too. Ditto fonts.c
pango_script_for_lang too.
pango_script_for_lang is bug 168900 :)
Created attachment 38122 [details] [review] Add consts to break.c Attaching patch to constify all but one of the structures in break.c.
Created attachment 38146 [details] [review] Make pango_script_for_lang constant pango_script_for_lang is a 3540 byte constant array, patch to make it constant and thus is .rodata.
*** Bug 168900 has been marked as a duplicate of this bug. ***
I duped 168900 to this one as comment #6's attachment patches the header file too.
Whoops. Note to self: get more sleep.
Comment on attachment 38122 [details] [review] Add consts to break.c static const, not const static, please. (But see overall comment)
Comment on attachment 38106 [details] [review] proposed patch Can you give stats on what the size increase is for the char * to char array is? It's conceivable that we might need to go to integer-offsets into a big string instead.
These patches would be somewhat more useful if someone had actually gone through Pango *comprehensively* and identified all spots rather than just picking off the biggest ones. (Pretty easy to do with objdumpa and/or nm) If someone does that, I'd like to see one single patch which covers all additions of 'const' rather than many small patch.
Changing char* to char[] increases the array size from 6k to 18k according to objdump
pango_script_for_lang is 0xDD4 bytes before and after the patch.
That of course, isn't the interesting number since the strings are now part of the structure while they weren't before. The interesting number is the change in the total size of the library.
That's referring to Tommi's comment, not Ross's, of course. The strings were already part of the array for pango_script_for_lang().
Silly me... Numbers from size and ls of stripped library: text data bss dec ls filename before: 159655 17064 16 176735 179524 libpango-1.0.so after: 166371 11048 16 177435 181604 libpango-1.0.so +/-: 6716 -6016 0 700 2080
That number is a odd. Your change adds 20 bytes per record and then removes 10-12 bytes per record by no longer needing the string. (The strings in xColors average 9.8 bytes by my count. Alignment might increase the storage a bit from that.) However, by your count, the total increase is 700 bytes .. so less than 1 byte per string for 752 strings.
Don't forget the \0 and that the old structure had a pointer, and that said pointer means padding. Code size ought to go down, btw., with the new code.
Those are accounted for in my numbers above.
The number I get is: text data bss dec hex filename 8331 6016 4 14351 380f .libs/pango-color.o 20535 0 4 20539 503b .libs/pango-color.o So, 6188 byte increase. Which is about what I predicted above. Of course, wasting 6k globally to save 6k per process is a net win, but I think we can do better. <implements> text data bss dec hex filename 12852 0 4 12856 3238 .libs/pango-color.o I'll attach the patch in a second.
Created attachment 38276 [details] [review] My patch Here's the patch I used. I changed the color table to be generated from rgb.txt, though I'm also checking in the generated table to avoid having to distribute rgb.txt and use perl in the build process.
OK, did a pretty comprehensive review and added const everwhere applicable. Change for libpango-1.8.so.0 Before After Change .text 102336 102336 +4 .rodata 52832 63700 +10868 .data.rel.ro 3204 3492 +288 .data 12216 16 -12200 So, ~12k less per-process data. Not a huge amount, but there are 34 processes on my system at the moment linked to Pango, so that's 300k. 2005-03-04 Owen Taylor <otaylor@redhat.com> Reduce non-shared data (#168899, inspired by patches from Tommi Komulainen and Ross Burton) * pango/pango-color.c pango/pango-color-table.h tools/gen-color-table.pl: Redo storage of colors to use offsets into a static string rather than embedded strings. (Inspired by a patch from Tommi Komulainen, #168899) * pango/break.c pango/fonts.c pango/pango-color.c pango/pango-layout.c pango/pango-markup.c pango/pango-script-lang-table.h pango/mini-fribidi/fribidi_types.c tools/gen-script-for-lang.c: Add const in various places * modules/arabic/arabic-fc.c modules/hebrew/hebrew-fc.c: modules/indic/{indic-fc.c,indic-ot-class-tables.c, indic-ot.h} modules/syriac/syriac-ot.c (syriac): Further addition of const.