Bug 25382

Summary: performance enhancements for <c:out> tag
Product: Taglibs Reporter: Mike Skells <mike.skells>
Component: Standard TaglibAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: enhancement CC: sunsear
Priority: P3    
Version: 1.0   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: modified OutSUport.java file (not a patch cvs diff is broken on my machine). based on standard 1.0 branch
cvs path file
full source of change
client app to generate performnace results
test jsp page to generate results

Description Mike Skells 2003-12-10 08:34:03 UTC
I was profiling an application and found that there was a lot of CPU being used 
as a result of the OutSupport handling of a string.

The current handler prints each individual character to the JspWriter.

with tomcat 4.0.6 the JspWriterImpl then converts every character to a String 
and writes it.

On my test system (running a development application) the application was 
consuming 15.6% of the CPU running OutSupport.out(..) and generating over 13000 
temporary object per page. Time for the Garbage collection is not included.

These figures were obtained with 
Jprobe 5.0.1
Tomcat 4.0.6 (very old I know)
taglibs-standard 1.0.4
Windows 2000
jdk 1.4.2_02

I enclose a patch that required no objects to be generated and reduced the CPU 
load to 2.3%, and no additional GC overhead 

this is achieved by writing blocks of characters between the 'special' 
characters, and coping with the special characters if and when they occur
Comment 1 Mike Skells 2003-12-10 08:43:44 UTC
Created attachment 9485 [details]
modified OutSUport.java file (not a patch cvs diff is broken on my machine). based on standard 1.0 branch
Comment 2 Pierre Delisle 2003-12-13 00:19:26 UTC
Mike,

Thanks for the patch. Indeed, it looks like your fix would provide some
substantial performance improvement. A few comments:
  - To minimize risk, I'll wait to commit any performance improvement once 1.1
final is released (should hopefully happen next week).
  - I'd also like us to implement some tests along with these fixes to make sure
any performance fix won't cause any regression. I'll look into integrating
JUnit/Cactus.
  - Would be nice to have some performance benchmarks we could run to document
these performance improvements. If you have something you could contribute
there, that would be great.

Thanks again.
  
    
Comment 3 Mike Skells 2003-12-15 08:41:32 UTC
I have no more performance information than I have already given, ie 15.3% -> 
2.3% based on a custommer application this is currently going through the birth 
pains.

The figures were based in JProbe, not Hotspot, but I cant think that the patch 
would make things worse

IMPORTANT
this patch is for JSTL 1.0 only. This file has been reworked for JSTL 1.1, and 
therefore the delay until JSTL 1.1 final is not relivent (apart from your 
workload!). Is there any planned releases for 1.0?

I dont have a Tomcat 5 installed and therefore I was not able to produce a 
patch for JSLT1.1, although the modifications should be equally applicable for 
the (HEAD) JSTL1.1 version of the file, but this would require some other 
changes, as the structure of the conversion process has changed minorly, to 
support Readers etc.

I dont believe that I will be able to contribute anything until after Xmas due 
to pressure of work. I am not sure what for this would take anyway, as it is 
dependent on JSPWriter implementation, which means that it requires to run in a 
servlet enviroment, so would it be a web page with a few <c:out> tags and a 
timer?
Comment 4 Felipe Leme 2004-05-12 00:27:06 UTC
CC'ing the taglibs-dev address to all Standard bugs. 
Comment 5 Mike Skells 2004-05-12 07:10:09 UTC
I am moving a product to JSTL 1.1 this week. 
I will port the patch I delivered for 1.1 this week, and attach it to this issue
Comment 6 Felipe Leme 2004-05-12 08:57:35 UTC
Hi Mike,

Yes, please fill a new bug, preferably with instructions on how to reproduce it.

If we later realize the new bug is caused by the same problem , we can mark it
then as duplicated of this one.

Thanks,

Felipe
Comment 7 Felipe Leme 2004-05-12 09:00:10 UTC
Oops, I put the comment on the wrong bug :-(

Anyway, in order to backport the fix to 1.0, please use the following CVS tag:

STANDARD_1_0_BRANCH

Felipe
Comment 8 Mike Skells 2004-05-12 11:15:58 UTC
the fix that I filed was against 1.0. I will produce a new one for 1.1 
(sometime ...)

I am not a committer for this project (happy to become one)
Comment 9 Felipe Leme 2004-05-13 03:36:53 UTC
Hi Mike,

Sorry, my previous message was posted to the wrong bug. 

Anyway, if you want to become a committer, you're in the right path:
contributing to the project as a developer, fixing bugs and submitting patches,
etc...

Regards,

Felipe

PS: here are some links that explain the Apache/Jakarta philosophy:

http://apache.org/foundation/how-it-works.html
http://jakarta.apache.org/site/getinvolved.html
http://jakarta.apache.org/taglibs/index.html#GettingInvolved
Comment 10 Mike Skells 2004-05-13 13:43:43 UTC
Created attachment 11530 [details]
cvs path file
Comment 11 Mike Skells 2004-05-13 13:44:28 UTC
Created attachment 11531 [details]
full source of change
Comment 12 Mike Skells 2004-05-13 13:46:07 UTC
Created attachment 11535 [details]
client app to generate performnace results
Comment 13 Mike Skells 2004-05-13 13:46:59 UTC
Created attachment 11537 [details]
test jsp page to generate results
Comment 14 Mike Skells 2004-05-13 13:51:45 UTC
as promised a patch for jstl 1.1.

The results of the patch are approx 5 times faster for non data containing no 
special characters, and 30% fastr for data containing mostly special characters

the test config for the results are 
tomcat 5.0.19
java 1.4.1_02
windows XP pro
AMD athlon 1.533Ghz

I attach a path, the actual source, a JSP test page, and a simple client that 
can run the test page.

The client app is only because IE cannot cope with volume of the output from 
the test page!

here are the results for operations
col 1 - text without special chars
col 2 - text without special chars - escapeXML=false
col 3 - text with some special chars
col 4 - text with some special chars - escapeXML=false
col 5 - text mostly special chars
col 6 - text mostly special chars - escapeXML=false

without patch

test results
1204,   47,     344,    31,     250,    47,     
296,    31,     265,    16,     250,    16,     
282,    31,     266,    47,     235,    31,     
422,    47,     391,    16,     375,    31,     
265,    31,     328,    31,     234,    32,     
281,    32,     297,    31,     250,    31,     
375,    31,     453,    31,     328,    31,     
282,    47,     297,    32,     313,    31,     
265,    31,     265,    31,     250,    32,     
375,    31,     438,    31,     234,    31,     
282,    32,     265,    31,     375,    78,     
265,    31,     453,    32,     234,    94,     
266,    47,     266,    15,     250,    31,     
390,    31,     250,    31,     235,    31,     
266,    31,     266,    32,     359,    31,     
266,    31,     390,    31,     235,    32,     
390,    32,     266,    31,     250,    31,     
282,    47,     266,    78,     359,    31,     
328,    31,     375,    94,     250,    16,     
406,    78,     265,    31,     234,    31,     
======================================================
Averages
359,    37,     320,    35,     275,    35,     
End of results




with patch

test results
62,     47,     94,     31,     219,    31,     
109,    31,     94,     31,     172,    31,     
63,     31,     125,    31,     187,    31,     
78,     31,     62,     32,     172,    32,     
63,     32,     79,     15,     172,    31,     
62,     15,     62,     31,     203,    31,     
63,     32,     63,     32,     188,    16,     
62,     31,     62,     31,     203,    31,     
63,     31,     78,     31,     172,    31,     
46,     31,     63,     31,     187,    32,     
63,     32,     62,     32,     172,    31,     
47,     31,     63,     31,     219,    31,     
62,     31,     62,     31,     172,    47,     
63,     31,     63,     16,     187,    31,     
47,     32,     62,     31,     172,    31,     
62,     31,     78,     31,     188,    16,     
47,     31,     94,     32,     203,    31,     
94,     31,     63,     31,     187,    32,     
47,     32,     62,     31,     188,    31,     
78,     46,     63,     31,     172,    47,     
======================================================
Averages
64,     32,     72,     29,     186,    31,     





Comment 15 Mike Skells 2004-05-13 13:55:03 UTC
I notice that ../comm/core/Util has a escapeXML method. ANy ideas where that is 
used?
Comment 16 Justyna Horwat 2004-05-13 22:58:08 UTC
The Util.escapeXml(String) method is used in Functions.escapeXml(String). That function simply escapes 
characters that could be interpreted as XML markup.
Comment 17 Mike Skells 2004-05-13 23:12:37 UTC
So I presume that the same performance improvements could be applied there, if 
this patch is accepted
Comment 18 Justyna Horwat 2004-06-11 01:44:10 UTC
*** Bug 26826 has been marked as a duplicate of this bug. ***
Comment 19 Justyna Horwat 2004-06-11 01:53:41 UTC
Using your patch, wrote a generic xml escaping method in the Util class. Found this to be about 5x 
faster than the existing implementation.

The out tag handler customized implementation that writes to the JspWriter did have some performance 
gains over the generic implementation and so kept the customized implementation for the tag handler.
Comment 20 Mike Skells 2004-06-11 08:14:57 UTC
The changes need to be applied to the standard 1_0 branch. As far as I can see 
they seem to be applied to the 1.1 main line only

This bug was raised on the 1.0 line, and contains fixes for the 1.0 and 1.1 line
Comment 21 Justyna Horwat 2004-06-11 21:26:58 UTC
Applied patch to Standard 1.0.x branch.