Bug 51149

Summary: [PATCH] slowdown in PNG processing when switching from JDK 1.6.0_17 to 1.6.0_18
Product: XMLGraphicsCommons - Now in Jira Reporter: Ognjen Blagojevic <ognjen.d.blagojevic>
Component: image codecsAssignee: XML Graphics Project Mailing List <general>
Status: RESOLVED FIXED    
Severity: critical    
Priority: P2    
Version: 1.4   
Target Milestone: --   
Hardware: PC   
OS: All   
Bug Depends on:    
Bug Blocks: 51465, 53055    
Attachments: Example image, intentionally blurred
this patch speeds up performance for certain kinds of PNG images
simple *.fo test that uses the image provided

Description Ognjen Blagojevic 2011-05-04 10:16:18 UTC
Created attachment 26958 [details]
Example image, intentionally blurred

This issue is discussed at the thread with the same name:

http://mail-archives.apache.org/mod_mbox/xmlgraphics-fop-dev/201103.mbox/%3C4D7A0D5B.4010500@gmail.com%3E


Summary:

PNG image sized 2035x1875 pixels, with color profile "sRGB IEC61966-2.1", takes 2s to process at Sun/Oracle JDK 1.6.0_17 and 40s at 1.6.0_18 or later.

Please read thread for details.
Comment 1 patches 2011-06-15 13:31:15 UTC
Hello, 
I have a similar bug (fop 1.0), but it doesn't appear in the version 1.3 of xmlgraphics (fop 0.95).
The problem comes from color model associated with the image.
* if is_sRGB_stdStale = true then getRBG method is very fast
* if is_sRGB_stdStale = false then getRBG  is slow because the treatment is delegated to ICC_ColorSpace.toRGB

The color model is correct, but it is modified by the class org.apache.xmlgraphics.image.loader.impl.imageio.ImageLoaderImageIO
by the following code (v 1.4): 
            if (providerIgnoresICC && cm instanceof ComponentColorModel) {
                // Apply ICC Profile to Image by creating a new image with a new
                // color model.
                ICC_Profile iccProf = tryToExctractICCProfile(iiometa);
                if (iccProf != null) {
                    ColorModel cm2 = new ComponentColorModel(
                            new ICC_ColorSpace(iccProf), cm.hasAlpha(), cm
                                    .isAlphaPremultiplied(), cm
                                    .getTransparency(), cm.getTransferType());
                    WritableRaster wr = Raster.createWritableRaster(imageData
                            .getSampleModel(), null);
                    imageData.copyData(wr);
                    BufferedImage bi = new BufferedImage(cm2, wr, cm2
                            .isAlphaPremultiplied(), null);
                    imageData = bi;
                    cm = cm2;

I removed this code and it works fine now, but it's dirty.

Until a fix, best regards.
Comment 2 Chris Bowditch 2012-02-07 08:25:19 UTC
We believe this issue was caused by the following change to the JDK:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6549882
Comment 3 Glenn Adams 2012-04-04 22:47:44 UTC
not an FOP bug; upgrade your Java platform to include fix
Comment 4 Chris Bowditch 2012-04-10 16:00:02 UTC
Hi Glenn,

Sorry but I don't agree with closing this bug just yet. Whilst its true that Java changed the implementation and that's what causes the slowdown, FOP still elects to use that particular JDK library. Luis has been working on a raw PNG Image Loader that addresses a couple of performance issues. The idea of the new loader is that it doesn't use the JDK library. 

I would like to leave it open and when Luis returns from vacation in a couple of days he can test out his new raw PNG handler to see if it alleviates this particular performance issue.

Thanks,

Chris
Comment 5 Glenn Adams 2012-04-10 17:25:29 UTC
(In reply to comment #4)
> Sorry but I don't agree with closing this bug just yet. Whilst its true that
> Java changed the implementation and that's what causes the slowdown, FOP still
> elects to use that particular JDK library. Luis has been working on a raw PNG
> Image Loader that addresses a couple of performance issues. The idea of the new
> loader is that it doesn't use the JDK library. 
> 
> I would like to leave it open and when Luis returns from vacation in a couple
> of days he can test out his new raw PNG handler to see if it alleviates this
> particular performance issue.

ok, sounds good
Comment 6 Luis Bernardo 2012-06-01 13:37:24 UTC
Created attachment 28871 [details]
this patch speeds up performance for certain kinds of PNG images

this patch addresses the issue described here: http://old.nabble.com/change-to-PNGImageReader-in-jdk1.6_14-impacts-performance-with-some-PNG-images-td33271018.html#a33271018.

Note: the approach implemented in this patch was suggested by Jeremias. The implementation and any mistakes, are mine.
Comment 7 Luis Bernardo 2012-06-01 13:42:46 UTC
Created attachment 28872 [details]
simple *.fo test that uses the image provided

These are some simple time measurements before and after the patch is applied:

BEFORE:
$ time /home/lbernardo/workspace/apachefop/fop -fo test.fo -pdf test.pdf
01-Jun-2012 14:16:51 org.apache.fop.events.LoggingEventListener processEvent
INFO: Rendered page #1.

real	0m19.934s
user	0m20.157s
sys	0m0.172s
$ ls -l test.pdf 
-rw-rw-r-- 1 lbernardo lbernardo 264239 2012-06-01 14:16 test.pdf

AFTER:
$ time /home/lbernardo/workspace/apachefop/fop -fo test.fo -pdf test.pdf
01-Jun-2012 14:17:32 org.apache.fop.events.LoggingEventListener processEvent
INFO: Rendered page #1.

real	0m3.749s
user	0m4.016s
sys	0m0.076s

$ ls -l test.pdf 
-rw-rw-r-- 1 lbernardo lbernardo 264239 2012-06-01 14:17 test.pdf
Comment 8 Luis Bernardo 2012-06-01 14:03:11 UTC
note: although not part of this patch, I am finishing up a raw PNG image loader that bypasses decoding. since this was mentioned here, the time measurement with this new image loader for the same example is also shown here:

$ time fop -c fop.xconf -fo test.fo -pdf test.pdf 
01-Jun-2012 14:49:04 org.apache.fop.apps.FopFactoryConfigurator configure
INFO: Default page-height set to: 11in
01-Jun-2012 14:49:04 org.apache.fop.apps.FopFactoryConfigurator configure
INFO: Default page-width set to: 8.26in
01-Jun-2012 14:49:05 org.apache.fop.events.LoggingEventListener processEvent
INFO: Rendered page #1.

real	0m0.659s
user	0m0.816s
sys	0m0.044s
$ ls -l test.pdf 
-rw-rw-r-- 1 lbernardo lbernardo 202386 2012-06-01 14:49 test.pdf

The size of the resulting file is smaller because the image (the IDAT chunk) is not decompressed.
Comment 9 Glenn Adams 2012-06-01 19:58:07 UTC
applied patch at http://svn.apache.org/viewvc?rev=1345323&view=rev

thanks luis, please review and close if satisfied
Comment 10 Ognjen Blagojevic 2012-08-15 14:54:53 UTC
Luis, thank you for your patch.

I tested and got similar results as Luis:

FOP 1.0    + Oracle JVM 1.6.0_24 is around 4 times slower than
FOP 1.1rc1 + Oracle JVM 1.6.0_24, but that one is also around 8 times slower than
FOP 1.0    + Oracle JVM 1.6.0_17.

In other words, there is some speed improvement (see comment 7), but in order to get the same speed as earlier with JDK 1.6.0_17, I guess we should wait for patch announced at comment 8.

Therefore, if no one object, I will reopen this issue.
Comment 11 Luis Bernardo 2012-08-15 21:38:23 UTC
the raw png image loader is part of the trunk now, but is disabled by default. see http://wiki.apache.org/xmlgraphics-fop/HowTo/ImageLoaderRawPNG for more info. the initial patch, the one used for measurements in comment 8, used it. that patch still ignores color profiles. there is a more recent patch, not applied yet, AFIK, that uses color profiles (see bug 40676). more information about color profiles can be found in bug 51465.
Comment 12 Ognjen Blagojevic 2012-08-16 09:25:59 UTC
Luis,

Thank you for your time to explain and work on patches on all bugs related to color profiles.

I believe you talk about attachment 29132 [details] on bug 40676, that is still not applied to the trunk. Should bug 40676 be reopened until this patch is applied?

Please, correct me if I'm wrong: 

XML Graphics pre-1.4, FOP pre-1.0 and Oracle Java pre-1.6.0_17 (or certain combinations of those) incorrectly ignored color profiles of PNG images, resulting in faster image processing and smaller PDF files. 

Latest versions of those (when patch 29132 is applied) will take color profile information into consideration, therefore taking longer to process images and generating larger PDF files.

The only way to use color profiles in PNG files but to keep both PDF size and performance as earlier, is to use ImageLoaderRawPNG, which will write PNG IDAT chunk stream directly into resulting PDF. That is, only if it is non-interlaced 8-bit-per-channel PNG.

Is this correct?
Comment 13 Luis Bernardo 2012-09-04 15:43:41 UTC
not quite correct...
- xmlgraphics pre 1.4 ignored color profiles; it was fast but in some cases the output would be wrong (but most of the time was correct because ignoring the color profile had no visible effect);
- with jdk 1.6.0_17, a new performance problem appeared but this was due to a change (a bug fix) inside the jdk that caused the xmlgraphics to take a different, and slower, path with some images; with the examples I checked the images had a color profile but I don't know whether that was relevant; a patch was submitted that tried to alleviate the performance impact, but the code was still slower than   with a jdk pre 1.6.0_17;
- the ImageLoaderRawPNG is faster in many situations (in particular with large images that use the Paeth filter) and the resulting file is usually smaller but not always, and that is particularly true after support for color profiles was added; the color profile is now (if different from the default one sRGB IEC61966-2.1) embedded in the output PDF which may result in larger file size; but if the color profile is the default one then I expect the ImageLoaderRawPNG to be faster and produce smaller file sizes in most cases when compared with the default ImageLoaderImageIO (with the caveat that only images with 8 bits per channel and without interlacing are supported);
- bug 40676 can be reopened but as the bug title says, the main issue of the bug was file size, not performance.
Comment 14 Ognjen Blagojevic 2012-10-08 07:47:50 UTC
Since the performance issue still exists (although mitigated to a certain degree), and since there is available patch at issue 40676 that solves it completely, I am reopening this issue.
Comment 15 Luis Bernardo 2012-10-20 23:55:50 UTC
outstanding patch for bug 40676 applied and issue status changed to resolved