'Re: [OpenJDK 2D-Dev] [8] request fro review: 8005530: [lcms] Improve performance of ColorConverOp fo'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [8] request fro review: 8005530: [lcms] Improve performance of ColorConverOp fo
From:       Jennifer Godinez <jennifer.godinez () oracle ! com>
Date:       2013-02-13 17:34:39
Message-ID: 511BCEAF.90706 () oracle ! com
[Download RAW message or body]

Hi Andrew,

Looks good.

Jennifer

On 2/1/2013 7:41 AM, Andrew Brygin wrote:
> Hello Phil,
> 
> please take yet another look at updated webrev:
> 
> http://cr.openjdk.java.net/~bae/8005530/webrev.02/
> 
> Summary of changes:
> - LCMS.c: unused native method getTagSize() was removed.
> 
> - mapfiles for lcms library are updated accordingly to changes in 
> native methods of LCMS
> (affects new and old build systems, is visible on solaris)
> 
> Thanks,
> Andrew
> 
> On 1/29/2013 1:52 AM, Phil Race wrote:
> > This is all fine by me.
> > 
> > -phil.
> > 
> > On 1/28/2013 1:40 AM, Andrew Brygin wrote:
> > > Hello Phil,
> > > 
> > > I have updated the fix according to your comment:
> > > 
> > > http://cr.openjdk.java.net/~bae/8005530/webrev.01/
> > > 
> > > Beside this, I have included some improvements related to
> > > access to profile header (which is quite important in case
> > > of decoding jpeg images with embedded color profile),
> > > and to raster processing.
> > > New benchmark to estimate the performance of image
> > > decoding was also added to the fix.
> > > 
> > > Please take a look to a comparison of performance level
> > > below.
> > > 
> > > Regarding to the change in build files: I have verified that
> > > it works for both old and new build systems. I am pretty
> > > sure that the change is safe (the HIGHEST level of optimization
> > > is already used for other libraries in jdk).
> > > 
> > > Would you like to see the change in build systems as a
> > > separate fix?
> > > 
> > > Thanks,
> > > Andrew
> > > 
> > > ======================================================================
> > > Options common across all tests:
> > > cmm.colorconv.ccop.ccopOptions.content=photo
> > > cmm.colorconv.ccop.ccopOptions.srcType=BYTE_3BYTE_BGR
> > > cmm.colorconv.embed.embedOptions.Images=MEDIUM
> > > cmm.opts.profiles=1001
> > > 
> > > cmm.colorconv.ccop.op_draw,cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=250: \
> > >  
> > > OpenJDK Baseline: 0.008438818 (var=18.29%) (100.0%)
> > > *************************************** |
> > > ****************************************|
> > > ****************************************|
> > > OpenJDK Fix: 0.012336448 (var=7.36%) (146.19%)
> > > ****************************************|******************
> > > ****************************************|*****************
> > > ****************************************|****************
> > > cmm.colorconv.ccop.op_draw,cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > >  
> > > OpenJDK Baseline: 0.900596556 (var=2.93%) (100.0%)
> > > ******************************************************|
> > > ***************************************************** |
> > > ******************************************************|
> > > OpenJDK Fix: 0.193663771 (var=7.52%) (110.1%)
> > > ******************************************************|*****
> > > ******************************************************|**
> > > ******************************************************|*****
> > > cmm.colorconv.ccop.op_draw,cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=250: \
> > >  
> > > OpenJDK Baseline: 0.009548438 (var=16.26%) (100.0%)
> > > ***********************************************|
> > > ***********************************************|
> > > *********************************************  |
> > > OpenJDK Fix: 0.011871784 (var=7.76%) (124.33%)
> > > ***********************************************|************
> > > ***********************************************|***********
> > > ***********************************************|********
> > > cmm.colorconv.ccop.op_draw,cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > >  
> > > OpenJDK Baseline: 0.927800441 (var=7.74%) (100.0%)
> > > ****************************************************|
> > > ****************************************************|
> > > ****************************************************|
> > > OpenJDK Fix: 0.247631935 (var=12.63%) (110.92%)
> > > ****************************************************|*****
> > > ****************************************************|*****
> > > ****************************************************|*****
> > > cmm.colorconv.ccop.op_img,cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=250: \
> > >  
> > > OpenJDK Baseline: 0.171872303 (var=13.62%) (100.0%)
> > > ******************************************|
> > > ******************************************|
> > > ******************************************|
> > > OpenJDK Fix: 0.238933601 (var=3.17%) (139.02%)
> > > ******************************************|****************
> > > ******************************************|****************
> > > ******************************************|****************
> > > cmm.colorconv.ccop.op_img,cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > >  
> > > OpenJDK Baseline: 0.615214994 (var=7.92%) (100.0%)
> > > ******************************|
> > > ******************************|
> > > ******************************|
> > > OpenJDK Fix: 0.001296456 (var=2.15%) (195.98%)
> > > ******************************|*****************************
> > > ******************************|****************************
> > > ******************************|*****************************
> > > cmm.colorconv.ccop.op_img,cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=250: \
> > >  
> > > OpenJDK Baseline: 0.065811677 (var=6.25%) (100.0%)
> > > ****************|
> > > ****************|
> > > ****************|
> > > OpenJDK Fix: 0.236734693 (var=7.65%) (359.72%)
> > > ****************|*****************************************
> > > ****************|******************************************
> > > ****************|******************************************
> > > cmm.colorconv.ccop.op_img,cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > >  
> > > OpenJDK Baseline: 0.763357432 (var=1.53%) (100.0%)
> > > ********|
> > > ********|
> > > ********|
> > > OpenJDK Fix: 0.001278227 (var=3.47%) (724.88%)
> > > ********|**************************************************
> > > ********|*************************************************
> > > ********|**************************************************
> > > cmm.colorconv.ccop.op_rst,cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=250: \
> > >  
> > > OpenJDK Baseline: 0.110134436 (var=1.92%) (100.0%)
> > > ***************************|
> > > ***************************|
> > > ***************************|
> > > OpenJDK Fix: 0.234074823 (var=4.62%) (212.54%)
> > > ***************************|******************************
> > > ***************************|*******************************
> > > ***************************|******************************
> > > cmm.colorconv.ccop.op_rst,cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > >  
> > > OpenJDK Baseline: 0.391746749 (var=16.2%) (100.0%)
> > > ****************|
> > > ****************|
> > > *************** |
> > > OpenJDK Fix: 0.001300390 (var=2.29%) (383.4%)
> > > ****************|*******************************************
> > > ****************|*******************************************
> > > ****************|******************************************
> > > cmm.colorconv.ccop.op_rst,cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=250: \
> > >  
> > > OpenJDK Baseline: 0.110379746 (var=1.38%) (100.0%)
> > > ***************************|
> > > ***************************|
> > > ***************************|
> > > OpenJDK Fix: 0.238619309 (var=3.32%) (216.18%)
> > > ***************************|******************************
> > > ***************************|******************************
> > > ***************************|*******************************
> > > cmm.colorconv.ccop.op_rst,cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > >  
> > > OpenJDK Baseline: 0.846569883 (var=10.33%) (100.0%)
> > > *************|
> > > *************|
> > > *************|
> > > OpenJDK Fix: 0.001290600 (var=2.88%) (453.39%)
> > > *************|*********************************************
> > > *************|**********************************************
> > > *************|*********************************************
> > > cmm.colorconv.embed.embd_img_read:
> > > OpenJDK Baseline: 0.299874002 (var=7.04%) (100.0%)
> > > ************|
> > > ************|
> > > ************|
> > > OpenJDK Fix: 0.003114456 (var=9.07%) (494.37%)
> > > ************|*********************************************
> > > ************|**********************************************
> > > ************|**********************************************
> > > cmm.profiles.getHeader:
> > > OpenJDK Baseline: 136.4378577 (var=4.34%) (100.0%)
> > > *|
> > > *|
> > > *|
> > > OpenJDK Fix: 6858.467330 (var=1.36%) (5026.81%)
> > > *|**********************************************************
> > > *|**********************************************************
> > > *|**********************************************************
> > > cmm.profiles.getNumComponents:
> > > OpenJDK Baseline: 135.4737465 (var=2.49%) (100.0%)
> > > *|
> > > *|
> > > *|
> > > OpenJDK Fix: 6903.416838 (var=4.1%) (5095.76%)
> > > *|*********************************************************
> > > *|********************************************************
> > > *|********************************************************
> > > 
> > > Summary:
> > > OpenJDK Baseline:
> > > Number of tests:  15
> > > Overall average:  18.159329310271605
> > > Best spread:      1.38% variance
> > > Worst spread:     18.29% variance
> > > (Basis for results comparison)
> > > 
> > > OpenJDK Fix:
> > > Number of tests:  15
> > > Overall average:  917.5243389674637
> > > Best spread:      1.36% variance
> > > Worst spread:     12.63% variance
> > > Comparison to basis:
> > > Best result:      5095.76% of basis
> > > Worst result:     110.1% of basis
> > > Number of wins:   15
> > > Number of ties:   0
> > > Number of losses: 0
> > > 
> > > 
> > > On 1/17/2013 1:54 AM, Phil Race wrote:
> > > > The variables
> > > > 154 ShortComponentRaster shortRaster;
> > > > 155 IntegerComponentRaster intRaster;
> > > > 156 ByteComponentRaster byteRaster;
> > > > 
> > > > are each used only in a localised few lines of code,
> > > > can we move them to the block where they are used so
> > > > that their scope is limited to that ? I don't know for a fact
> > > > but I can imagine that the VM can then skip allocating stack for them
> > > > until you enter that block which actually needs them.
> > > > 
> > > > As you note,this takes into account old and new builds in the 
> > > > makefile changes
> > > > Have you tested both ?
> > > > And unless you are 100% confident in this [minimal] change
> > > > you should get sign off from the build group as they've recently 
> > > > requested.
> > > > 
> > > > You should add "noreg-perf" as a label in the JIRA
> > > > 
> > > > -phil.
> > > > 
> > > > On 1/9/2013 4:25 AM, Andrew Brygin wrote:
> > > > > Hello Jennifer and Phil,
> > > > > 
> > > > > could you please review a fix for CR 8005530?
> > > > > 
> > > > > CR:  http://bugs.sun.com/view_bug.do?bug_id=8005530
> > > > > Webrev:http://cr.openjdk.java.net/~bae/8005530/webrev.00/
> > > > > 
> > > > > This fix improves performance of ColorConvertOp.filter()
> > > > > operation in case of lcms. The fix can be divided into
> > > > > three separate changes:
> > > > > 
> > > > > * provide support for custom component images in
> > > > > LCMSImageLayout.
> > > > > This change affects the case of conversion to default
> > > > > destination.
> > > > > 
> > > > > * provide a way to process whole image, instead of
> > > > > scan-by-scan processing, if both source and
> > > > > destination images do not contain padding samples
> > > > > (i.e. next scan starts immediately after previous).
> > > > > 
> > > > > * increase optimization level for lcms library from
> > > > > LOW to HIGHEST. This change affects both new and
> > > > > standard build systems.
> > > > > 
> > > > > A benchmark comparison below illustrates the increase
> > > > > of performance:
> > > > > 
> > > > > Options common across all tests:
> > > > > testname=cmm.colorconv.ccop.op_img
> > > > > cmm.colorconv.ccop.ccopOptions.srcType=BYTE_3BYTE_BGR
> > > > > cmm.colorconv.ccop.ccopOptions.content=photo
> > > > > cmm.opts.profiles=1001
> > > > > 
> > > > > cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=250: \
> > > > >  
> > > > > OpenJDK Baseline: 0.152008134 (var=1.02%) (100.0%)
> > > > > *************************************|
> > > > > *************************************|
> > > > > *************************************|
> > > > > OpenJDK Fix: 0.245116358 (var=2.0%) (161.25%)
> > > > > *************************************|**********************
> > > > > *************************************|**********************
> > > > > *************************************|*********************
> > > > > cmm.colorconv.ccop.ccopOptions.dstType=BYTE_3BYTE_BGR,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > > > >  
> > > > > OpenJDK Baseline: 0.914826498 (var=0.41%) (100.0%)
> > > > > ***************************|
> > > > > ***************************|
> > > > > ***************************|
> > > > > OpenJDK Fix: 0.001310043 (var=2.24%) (221.48%)
> > > > > ***************************|********************************
> > > > > ***************************|********************************
> > > > > ***************************|********************************
> > > > > cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=250: \
> > > > >  
> > > > > OpenJDK Baseline: 0.060737151 (var=1.57%) (100.0%)
> > > > > ***************|
> > > > > ***************|
> > > > > ***************|
> > > > > OpenJDK Fix: 0.242736486 (var=1.53%) (399.65%)
> > > > > ***************|*******************************************
> > > > > ***************|********************************************
> > > > > ***************|********************************************
> > > > > cmm.colorconv.ccop.ccopOptions.dstType=COMPATIBLE_DST,cmm.colorconv.ccop.ccopOptions.size=4000: \
> > > > >  
> > > > > OpenJDK Baseline: 0.559251559 (var=0.7%) (100.0%)
> > > > > *******|
> > > > > *******|
> > > > > *******|
> > > > > OpenJDK Fix: 0.001306904 (var=1.05%) (838.16%)
> > > > > *******|***************************************************
> > > > > *******|****************************************************
> > > > > *******|****************************************************
> > > > > 
> > > > > Summary:
> > > > > OpenJDK Baseline:
> > > > > Number of tests:  4
> > > > > Overall average:  0.053373173444166644
> > > > > Best spread:      0.41% variance
> > > > > Worst spread:     1.57% variance
> > > > > (Basis for results comparison)
> > > > > 
> > > > > OpenJDK Fix:
> > > > > Number of tests:  4
> > > > > Overall average:  0.12261744826137347
> > > > > Best spread:      1.05% variance
> > > > > Worst spread:     2.24% variance
> > > > > Comparison to basis:
> > > > > Best result:      838.16% of basis
> > > > > Worst result:     161.25% of basis
> > > > > Number of wins:   4
> > > > > Number of ties:   0
> > > > > Number of losses: 0
> > > > > 
> > > > > Thanks,
> > > > > Andrew
> > > > > 
> > > > 
> > > 
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic