tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1643323 - in /tomcat/trunk: java/org/apache/catalina/connector/ java/org/apache/catalina/security/ java/org/apache/el/parser/ java/org/apache/jasper/ java/org/apache/jasper/compiler/ java/org/apache/jasper/runtime/ java/org/apache/jasper/s...
Date Sat, 06 Dec 2014 17:18:11 GMT
2014-12-06 19:37 GMT+03:00 Mark Thomas <markt@apache.org>:
> On 06/12/2014 16:07, Konstantin Kolinko wrote:
>> 2014-12-05 18:18 GMT+03:00  <markt@apache.org>:
>>> Author: markt
>>> Date: Fri Dec  5 15:18:46 2014
>>> New Revision: 1643323
>>>
>>> URL: http://svn.apache.org/r1643323
>>> Log:
>>> Replace System.getProperty("line.separator") with System.lineSeparator()
>>>
>>> Modified:
>>>     tomcat/trunk/java/org/apache/catalina/connector/CoyoteWriter.java
>>>     tomcat/trunk/java/org/apache/catalina/security/Constants.java
>>>     tomcat/trunk/java/org/apache/catalina/security/SecurityListener.java
>>>     tomcat/trunk/java/org/apache/el/parser/ParseException.java
>>>     tomcat/trunk/java/org/apache/jasper/Constants.java
>>>     tomcat/trunk/java/org/apache/jasper/compiler/AntCompiler.java
>>>     tomcat/trunk/java/org/apache/jasper/compiler/DefaultErrorHandler.java
>>>     tomcat/trunk/java/org/apache/jasper/compiler/ErrorDispatcher.java
>>>     tomcat/trunk/java/org/apache/jasper/compiler/JavacErrorDetail.java
>>>     tomcat/trunk/java/org/apache/jasper/runtime/BodyContentImpl.java
>>>     tomcat/trunk/java/org/apache/jasper/runtime/JspWriterImpl.java
>>>     tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
>>>     tomcat/trunk/java/org/apache/juli/JdkLoggerFormatter.java
>>>     tomcat/trunk/java/org/apache/juli/OneLineFormatter.java
>>>     tomcat/trunk/java/org/apache/juli/VerbatimFormatter.java
>>>     tomcat/trunk/java/org/apache/tomcat/buildutil/CheckEol.java
>>>     tomcat/trunk/test/org/apache/juli/TestClassLoaderLogManager.java
>>>
>>
>> A small bikeshed issue:
>>
>> I think in many places it is better to cache the value in a local
>> final variable inside a method rather than repeatedly call the getter.
>>
>> The first example in files changed by this commit:
>> /java/org/apache/el/parser/ParseException.java
>>
>> In Jasper there are several places where line separator is used
>> repeatedly in the same method and they can benefit from such local
>> variable, e.g.
>> /java/org/apache/jasper/compiler/DefaultErrorHandler.java
>
> Are you sure? I'd expect the complier to optimise this and therefore
> prefer cleaner code.

I am not sure, but I think that this is a good guess

1) The System.lineSeparator field that this getter reads is not final.

I am not sure how much of that can be optimized.

2) I think that if we avoid repeated method calls, it results in
smaller byte code generated for the method.

In general, if the call is repeated three times or more it looks odd to me.

3) I think that I have seen similar optimizations (local variables)
elsewhere in JRE code. I remember seeing local variable that was used
to cache a field value to avoid repeated field read.

4) PrintWriter of Java 8 still has final lineSeparator field in it.


Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message