213753 - [ps] Runtime bloat in gfx/src/ps/nsPostScriptObj.cpp from fprintf()s

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Kenneth Herron]

nsPostScriptObj.cpp contains a sequence of over 1300 calls to fprintf() each
printing a one-line string. This contributes around 32k to the runtime code size
on linux (about 45k on solaris) contrasted with using a single fprintf() call.

Comment 1

[Kenneth Herron]

Attachment: Convert to a single |fprintf| call.

This patch converts the entire block to a single |fprintf| call. The compiler
concats the individual strings into one big string.

This saves about 32k of text on linux (gcc 3.2):

  > size libgfx* nsPost*
     text	   data     bss     dec     hex filename
   265206	 217300      44  482550   75cf6 libgfxps.so
   299447	 217328      44  516819   7e2d3 libgfxps.so.old
    60862	   2032       8   62902    f5b6 nsPostScriptObj.o
    92644	   2032       8   94684   171dc nsPostScriptObj.o.old

and about 45k on solaris:

  > size libgfx* nsPostScriptObj.o*   
  libgfxps.so: 145320 + 233464 + 64 = 378848
  libgfxps.so.old: 190960 + 238440 + 64 = 429464
  nsPostScriptObj.o: 91887 + 2688 + 8 = 94583
  nsPostScriptObj.o.old: 199262 + 2688 + 8 = 201958

On my fairly wimpy solaris box, it also reduces the time to compile this file
from 8m15s using 120Mb to 0m20s using 20Mb.

Comment 2

[Kenneth Herron]

Comment on attachment 128443 [details] [diff] [review]
Convert to a single |fprintf| call.

I was asked in IRC how I produced the patch. It was basically three vi s//
operations. From memory:

s/^  fprintf(f, /    /
s/); *$//
s/\%\%/\%/g

i.e. fix up the beginning of each line, fix up the end, and un-double the %
characters. A couple of lines had to be cleaned up by hand because they didn't
match the first pattern properly.

The |fprintf| calls in question are all printing constant strings with no
variable values involved. Before making the change I printed a test page to a
file. After making the change I reprinted the page and compared the resulting
PS; there was no difference other than a timestamp which appears at the bottom
of each page.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 128443 [details] [diff] [review]
Convert to a single |fprintf| call.

sr=bzbarsky; once this has review, let me know and I can land it.

Comment 4

[tenthumbs]

At about line 610 you end up with 
  fprintf(f, "%s", huge_string);
Wouldn't
  fputs (huge_string, f);
be faster? There are actually lots of other places where this also 
occurs.

Comment 5

[Kenneth Herron]

I wrote a quickie benchmark that tests four different methods of writing a big
string and got results like the following:

fprintf(f, String)      17400876
fprintf(f, %s, String)  2297751
fputs(String, f)        2482240
fwrite(String, sizeof, sizeof, f)       115029

fprintf(f, String)      15610071
fprintf(f, %s, String)  3248828
fputs(String, f)        2884262
fwrite(String, sizeof, sizeof, f)       154088

fprintf(f, String)      16031274
fprintf(f, %s, String)  2507793
fputs(String, f)        2234064
fwrite(String, sizeof, sizeof, f)       115137

fprintf(f, String)      14915339
fprintf(f, %s, String)  2595806
fputs(String, f)        2410765
fwrite(String, sizeof, sizeof, f)       153214

Those numbers are time in microseconds to do the operation 100,000 times writing
to /dev/null. As you can see there's a big advantage going from

    fprintf(f, string)
to
    fprintf(f, "%s", string)

and another big advantage going to |fwrite|. |fprintf| vs. |fputs| tends to
favor |fputs| but the results aren't consistent.

That said, this code only runs once per document printed, so optimizing for
speed isn't the first priority. I used fprintf because I felt that

    fputs(
        [1314 lines]
        , f)

was bad style. fwrite is worse in this regard.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 128443 [details] [diff] [review]
Convert to a single |fprintf| call.

r=dbaron if you do a before-patch and after-patch printing tests, diff the two
resulting postscript files, and make sure they're the same.  (I didn't look too
closely.  My eyes would glaze over.)

Comment 7

[Kenneth Herron]

Attachment: Perl script to perform the conversion

Clearly, people aren't comfortable that the patch contains no mistakes. So I've
written this perl script to perform the change. You can run it as:

    perl ./convert-it nsPostScriptObj.cpp > new-file	# Writes to stdout
    perl -i.bak ./convert-it nsPostScriptObj.cpp	# Update in place

When run as above, the script will convert the block of code to a single
fprintf() call, identical to the patch. Alternately, you can add the switch
"-fwrite":

    perl ./convert-it -fwrite nsPostScriptObj.cpp > new-file

In this case it will convert the code to the form:

  static const char psBoilerplate[] = {
    "/rhc {\n"
    "	 {\n"
    "	     currentfile read {\n"
    [etc.]
    "  /unicodeshow2 { real_unicodeshow_native } bind def\n"
    "} bind def\n"

    "\n"
  };
  fwrite(psBoilerplate, 1, sizeof(psBoilerplate) - 1, f);

As noted in a previous comment, fwrite() is faster than fprintf() (though
again, this code only executes once per print job).

Whether using the patch or the perl script, the generated postscript is
identical. The following files were produced by printing the google home page
to a file using various versions of mozilla:

$ ls -l google*
-rw-r--r--    1 kherron  kherron    227780 Sep 20 13:01 google-orig.ps
-rw-r--r--    1 kherron  kherron    227780 Sep 20 13:05 google-patch.ps
-rw-r--r--    1 kherron  kherron    227780 Sep 20 13:12 google-perl-fprintf.ps
-rw-r--r--    1 kherron  kherron    227780 Sep 20 13:17 google-perl-fwrite.ps

$ md5sum google*
45c830db9ed9a39ec4567db4cb871118  google-orig.ps
45c830db9ed9a39ec4567db4cb871118  google-patch.ps
45c830db9ed9a39ec4567db4cb871118  google-perl-fprintf.ps
45c830db9ed9a39ec4567db4cb871118  google-perl-fwrite.ps

Mozilla print jobs normally contain the date/time in the margin; to avoid this
I went into File->Page Setup->Margins, then set the right footer to "blank".

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->kjh

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2003-09-20 14:27 -0700.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

  libgfxps.so
  	Total:	     -22736 (+0/-22736)
  	Code:	     -21040 (+0/-21040)
  	Data:	      -1696 (+0/-1696)
  	      -1696 (+0/-1696)	R (DATA)
  		      -1696 (+0/-1696)	UNDEF:libgfxps.so:R
  			        -20	kCharsetConverterManagerCID
  			      -1676	kFCSCID
  	     -21040 (+0/-21040)	T (CODE)
  		     -21040 (+0/-21040)	UNDEF:libgfxps.so:T
  			         -8	global constructors keyed to
                                            nsRenderingContextImpl::gBackbuffer
  			     -21032	nsPostScriptObj::begin_document(void)