85207 - [ps] postscript module too big due to inefficient static font tables

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[tor]

The postscript module has a number of static font metric tables for the standard
postscript fonts.  The structures storing this information (fontInformation and
AFM_Single_Char_Metrics) have many members of type double, when the only values
stored in them are integers in the 16-bit range.  Converting the structures to
use shorts should shrink the postscript module on the order of 150kb.

The font table headers have a comment that says they were generated by
afmgen.exe.  Do we have the source for that somewhere so we can modify
the output?

Comment 1

[Christopher Blizzard (:blizzard)]

dcone, what's that utility?

Comment 2

[dcone (gone)]

According to the Adobe Font Metrics File Format Specification.. those values are 
doubles (called numbers).  That is the reason I made them double.. just to 
follow those guidelines and be compatible with any future changes.  I would 
prefer to leave them as such.
The AFMFont utility is something I wrote to parse and make the font tables for 
our default fonts.

Comment 3

[tor]

While you probably want to keep a floating point representation at runtime so
you can load arbitrary afm files, the static default tables don't need this
precision and can be shrunk for compilation.

Comment 4

[dcone (gone)]

I would prefer to leave this in its current state for maintainability.  I would 
love to change them to ints if the Adobe Font Metrics File Format Specification 
gave us a way to do such a thing, but I proceed in this area with caution 
because there is so much about PostScript and the Internationalization issues I 
dont fully understand.

Comment 5

[tor]

Again, I'm not saying that the runtime representation has to change, just
that the default font tables don't use floating point and can be stored in
a more efficient manner prior to use.

Another possibility: since there is an afm parser in gfx/src/ps, could
we distribute the default font metrics as a jar of afm files instead of
compiling them in?  They should be fairly amenable to jar compression.

Comment 6

[dcone (gone)]

I am confused.. are you saying in the include file use ints for the structure 
initialization, the compiler will convert them to the doubles when it 
initializes the structure during the compile?

Comment 7

[dcone (gone)]

I really dont have time to fix this, I will mark as remind so it wont disapear 
but it will not be addressed for a very long time.

Comment 8

[Christopher Hoess (gone)]

REMIND is deprecated per bug 35839.

Comment 9

[Christopher Hoess (gone)]

->rods

Comment 10

[dcone (gone)]

this is mine

Comment 11

[Boris Zbarsky [:bzbarsky]]

just to clarify, alec, you were cced at my suggestion. I figured you would find
this bug very interesting, to say the least....

Comment 12

[Alec Flett]

here's my take: it sounds like we're talking about possibly storing these values
at compile-time in a smaller, less precise format and then inflating them at
runtime. 

Personally, I don't like this idea because what you end up doing is paying the
price of initialization twice - once to fault these pages into memory from the
DLL, and once when you initialize a new structure on the heap somewhere with the
inflated values. I see the same issue with keeping data in a .jar - you're still
paying the price of the data and by keeping it in a .jar all you've done is
moved the footprint impact from a .dll to a .jar. And likely, the data in the
.jar would be in a more expanded, verbose format and would actually take up a
larger footprint.

Maybe there is a middle road? Is there any way we can represent the data as
smaller values (i.e. 16-bit) and "inflate" them at runtime on an as-needed
basis? I mean without any sort of caching, etc. I don't know much about the use
of these values, but even if they are accessed often, it seems like it would be
ok to slow down ps printing slightly in order to win footprint. If they are only
accessed a few times for each print, and ps printing would slow down only
slightly, then it seems like even a better idea..

dcone, I understand your desire to keep this maintainable, but if we could
perhaps tweak your utility to output this special 16-bit format, maybe
maintainability would be the same?

Comment 13

[Kenneth Herron]

I'll take this. The AFM_Single_Char_Metrics structure stores nine values for
each glyph within a font, the character code, four character-width values, and
the character's bounding box. Mozilla only uses one of the width values and it
doesn't use the bounding box, so we could cut this down to the character code
and one width.

The character width is a floating-point value. It's the width of the character
for a 1pt version of the font, in units of 1/1000 point. There's no maximum
width, but widths over 1000 mean the character is wider than the font height,
which is unusual. I surveyed all of the AFM files I had on hand (306 different
fonts) and found a range from -56 through 2936. Many of the values had
fractional parts, but none needed more than 6 digits of accuracy.

The character code has a range of -1..255 so it could be stored in two bytes.
There can be more than 256 glyphs on a font, thus potentially more than 256
unique width values. This suggests two methods for storing this information:

1) Store a character code and a width for each character. This would take eight
bytes if the width were a float.

2) Store a table of unique widths for the font, along with a character code and
width table index for each character. This would be four bytes per character,
plus the size of the width table, which is worst case another float per
character. This would generally save memory, but the software which parses AFM
files would be more complex.

Whichever method was used, the header files containing the width metrics
(Times-Roman.h and so on) would have to be regenerated. nsAFMObject.cpp contains
some code to parse an AFM file and write a header, but it's not enough to build
a standalone program.

Comment 14

[Kenneth Herron]

Attachment: Simple space-saving patch

This patch stores AFM data using floats instead of doubles, which saves 96k of
space, and declares the data as const, which moves it out of the data segment
and into the text segment. It's not a comprehensive fix for this bug, but it's
a very simple change which doesn't require regenerating the AFM data files
(Times-Roman.h and the others). I've tested this with a gtk build of mozilla,
and it has absolutely no effect on text layout.

What I'd like to do is check this in now for the space savings benefit, and
hold off any more comprehensive change for the 1.9 cycle. It's likely that
cairo will deprecate all of this code, but if not, this old AFM font support is
about due for serious cleanup.

Comment 15

[rbs]

Comment on attachment 187653 [details] [diff] [review]
Simple space-saving patch

sr=rbs, sounds reasonable for a 96K win.

Comment 16

[Benjamin Smedberg]

Comment on attachment 187653 [details] [diff] [review]
Simple space-saving patch

a=bsmedberg for landing 6/30 only

Comment 17

[Kenneth Herron]

Space-saving patch checked in. Saved 98k:

  Overall Change in Size
  	Total:	     -98784 (+110144/-208928)
  	Code:	       -254 (+0/-256)
  	Data:	     -98530 (+110144/-208672)
  
  libgfxps.so
  	Total:	     -98784 (+110144/-208928)
  	Code:	       -254 (+0/-256)
  	Data:	     -98530 (+110144/-208672)

Leaving the bug open for possible additional work.

Comment 18

[Wayne Mery (:wsmwk)]

Kevin in comment #17:
> Leaving the bug open for possible additional work.

what additional work is needed?

Comment 19

[Kenneth Herron]

Resolving fixed. This was kept open for possible additional work. The code at issue is obsolete on the trunk, and only major bug fixes are being taken on the branches.