Bug 14444 - [PATCH] a performance patch for PDFInfo class
Summary: [PATCH] a performance patch for PDFInfo class
Status: CLOSED WONTFIX
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: pdf (show other bugs)
Version: all
Hardware: Other All
: P3 normal
Target Milestone: ---
Assignee: fop-dev
URL: http://www.rootshell.be/~hornyakl/dow...
Keywords:
: 15022 (view as bug list)
Depends on:
Blocks:
 
Reported: 2002-11-11 13:28 UTC by Laszlo Hornyak
Modified: 2012-04-01 13:52 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Hornyak 2002-11-11 13:28:41 UTC
Using stringbuffer for dynamicaly created string appendation may be faster.
pls mail me if you like patches like this. I could send lots.

thx.
Comment 1 Jeremias Maerki 2002-11-11 13:43:18 UTC
Please check that diff again. It seems to have a bug:
StringBuffer buf = new StrinfgBuffer();

Please run the compiler and test before submitting the patch. But yes, this 
kind of patch is interesting for us, especially because it's for the redesign.
Thanks.
Comment 2 Kevin O'Neill 2002-11-11 21:58:17 UTC
Sorry if this seems hard but this is the sort of performance enhancement
I was talking about yesterday. If people are going to do these sorts of
"enhancements" then they should be aware of the effects.

It's always easier to work with examples.

public class StringTest
{
    // String Buffer
    public String testStringBufferStraightCall()
    {
        StringBuffer sb = new StringBuffer();
        
        sb.append("this ");
        sb.append(makeString("is "));
        sb.append("a ");
        sb.append(makeString("test"));

        return sb.toString();
    }

    public String testStringBufferChained()
    {
        StringBuffer sb = new StringBuffer();

        sb.append("this ")
            .append(makeString("is "))
            .append("a ")
            .append(makeString("test"));

        return sb.toString();
    }

    public String testStringAdd()
    {
        return
            "this "
            + makeString("is ")
            + "a "
            + makeString("test");
    }

    public String testIncrement()
    {
        String result = "this ";
        result += makeString("is ");
        result += "a ";
        result += makeString("test.");

        return result;
    }

    private String makeString(String testString)
    {
        return testString;
    }
}

Which of the above is faster?

A simple timer test will show that testStringAdd() is the fastest,
followed closely by testStringBufferChained(). For the reason why, lets
look at the byte-code.

Method java.lang.String testStringBufferStraightCall()
   0 new #2 <Class java.lang.StringBuffer>
   3 dup
   4 invokespecial #3 <Method java.lang.StringBuffer()>
   7 astore_1
   8 aload_1
   9 ldc #4 <String "this ">
  11 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  14 pop
  15 aload_1
  16 aload_0
  17 ldc #6 <String "is ">
  19 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  22 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  25 pop
  26 aload_1
  27 ldc #8 <String "a ">
  29 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  32 pop
  33 aload_1
  34 aload_0
  35 ldc #9 <String "test">
  37 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  40 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  43 pop
  44 aload_1
  45 invokevirtual #10 <Method java.lang.String toString()>
  48 areturn

Method java.lang.String testStringBufferChained()
   0 new #2 <Class java.lang.StringBuffer>
   3 dup
   4 invokespecial #3 <Method java.lang.StringBuffer()>
   7 astore_1
   8 aload_1
   9 ldc #4 <String "this ">
  11 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  14 aload_0
  15 ldc #6 <String "is ">
  17 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  20 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  23 ldc #8 <String "a ">
  25 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  28 aload_0
  29 ldc #9 <String "test">
  31 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  34 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  37 pop
  38 aload_1
  39 invokevirtual #10 <Method java.lang.String toString()>
  42 areturn

Method java.lang.String testStringAdd()
   0 new #2 <Class java.lang.StringBuffer>
   3 dup
   4 invokespecial #3 <Method java.lang.StringBuffer()>
   7 ldc #4 <String "this ">
   9 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  12 aload_0
  13 ldc #6 <String "is ">
  15 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  18 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  21 ldc #8 <String "a ">
  23 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  26 aload_0
  27 ldc #9 <String "test">
  29 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  32 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  35 invokevirtual #10 <Method java.lang.String toString()>
  38 areturn

Method java.lang.String testIncrement()
   0 ldc #4 <String "this ">
   2 astore_1
   3 new #2 <Class java.lang.StringBuffer>
   6 dup
   7 invokespecial #3 <Method java.lang.StringBuffer()>
  10 aload_1
  11 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  14 aload_0
  15 ldc #6 <String "is ">
  17 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  20 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  23 invokevirtual #10 <Method java.lang.String toString()>
  26 astore_1
  27 new #2 <Class java.lang.StringBuffer>
  30 dup
  31 invokespecial #3 <Method java.lang.StringBuffer()>
  34 aload_1
  35 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  38 ldc #8 <String "a ">
  40 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  43 invokevirtual #10 <Method java.lang.String toString()>
  46 astore_1
  47 new #2 <Class java.lang.StringBuffer>
  50 dup
  51 invokespecial #3 <Method java.lang.StringBuffer()>
  54 aload_1
  55 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  58 aload_0
  59 ldc #11 <String "test.">
  61 invokespecial #7 <Method java.lang.String
makeString(java.lang.String)>
  64 invokevirtual #5 <Method java.lang.StringBuffer
append(java.lang.String)>
  67 invokevirtual #10 <Method java.lang.String toString()>
  70 astore_1
  71 aload_1
  72 areturn

So the answer is the compiler converts String additions to
StringBuffer.append(). Because it's done at the compiler level it can
also do optimizations that can't be done at the java code level (look at
the number of loads and pops). Quoting from the StringBuffer javadoc.

String buffers are used by the compiler to implement the binary string
concatenation operator +. For example, the code:

     x = "a" + 4 + "c"
 

is compiled to the equivalent of:

     x = new StringBuffer().append("a").append(4).append("c")
                           .toString()
 

So the first recommendation is to use String "+" for this type of
method, it's easier to read and runs faster.

So why would you use StringBuffer at all? Notice that the String
concatenation calls the no argument constructor for StringBuffer. On my
vm this means that a 16 char StringBuffer is allocated, this means in
the sample above it will grow at least once creating garbage and
allocating memory, both can be slow operations. So for large String
concatenation in PERFORMANCE SENSITIVE areas, create a StringBuffer of
at least the size you need by calling StringBuffer(SIZE) and use chained
StringBuffer calls. StringBuffers can carry a single instance through
loops, String concatenation can't.

Note: The hotspot vms are very hard to profile, especially in server
situations. You'll need to warm the code to ensure that hotspot has had
a chance to apply its optimizations.

This is in an area of code that is run once per invocation of the PDF
writer, there is no way this would show up as a hot spot in a profile.
It is better than += but it's harder to read. If these sort of
"optimizations" are going to be done, then they should be done in an
informed manor and using an understanding of the effect to create truly
optimized code rather than a sort of optimized mess. My opinion in this
case would be to convert it to use the "+" operator. It's not a
performance critical area, '"foo" + "bar"' is easier to read than
sb.append("foo").append("bar") and lastly the next version of the
compiler may have some new optimizations that take care of the sizing
issue.
Comment 3 Laszlo Hornyak 2002-11-12 12:20:10 UTC
Kevin,
The original code was lots of string += operations, which is simple but slow.
The patch replaces it with StringBuffer operations.

notes:
1. It didn`t became more difficult, did it?
2. It became faster. 14% with my test, but if you write your own test, it wont
be slower then before.
3. Yes, it is still not optimal. Maybe public "void toPDF(OutputStream)" could
be a more optimal solution.

Oh my god, it is sooo f*cking simple...
Comment 4 Oleg Tkachenko 2002-12-02 16:06:39 UTC
So, what should we do? Laszlo, can you please fix your diff as Jeremias suggested?
Comment 5 Oleg Tkachenko 2002-12-03 17:00:52 UTC
*** Bug 15022 has been marked as a duplicate of this bug. ***
Comment 6 Oleg Tkachenko 2002-12-03 17:01:50 UTC
See resubmitted patch in bug #15022.
Comment 7 Jeremias Maerki 2003-05-14 08:29:52 UTC
The URL is not valid anymore and there have been discussions about the pros 
and cons of these optimizations. So we'll just leave it at that for the 
moment. More pressing matters to follow. Closing entry...
Comment 8 Glenn Adams 2012-04-01 13:52:14 UTC
batch transition to closed remaining pre-FOP1.0 resolved bugs