Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Received-SPF: pass (nike.apache.org: local policy)
Message-ID: <4A3AD43D.2000709@hanik.com>
Date: Thu, 18 Jun 2009 17:56:45 -0600
From: Filip Hanik - Dev Lists <devlists@hanik.com>
User-Agent: Thunderbird 2.0.0.21 (X11/20090320)
MIME-Version: 1.0
To: Tomcat Developers List <dev@tomcat.apache.org>
Subject: Re: svn commit: r785952 - in
 /tomcat/trunk/test/org/apache/catalina/valves:
 	./ Benchmarks.java
References: <20090618083229.6E5DD2388892@eris.apache.org>
	 <4A3A1E9A.1010104@apache.org>
 <4A3A376B.9010908@apache.org>	 <25aac9fc0906180943j26b4694cn7e67afe6ed387036@mail.gmail.com>
 <25aac9fc0906180951i77ae25a3nbebb9785c80fd27b@mail.gmail.com>
In-Reply-To: <25aac9fc0906180951i77ae25a3nbebb9785c80fd27b@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

I think the entire Benchmark test is wrong.
I'm not sure what we are trying to do in this test, in my mind we are 
proving nothing with this test :)

The thread safety in AccessLogValve is the fact that the formatters are 
not thread safe and can yield some funky dates showing up.
And in the ideal solution its just not to wrap everything up in a 
synchronized statement.
 
The other thread safety issue in AccessLogValve is the the rotation of 
files, since it seems as one thread can close the file

There are more efficient AccessLogValve, instead of doing all this 
comparison crap on every single request, and writing to the file on 
every single request.

An example:
1. single back thread updates the currentDateString once a second.
2. Add the log entries to the queue, who writes out the buffer once a 
second.

If you don't want a background thread, then still the stuff going on in 
the Benchmark test is not needed, and the bench mark is far from 
efficient and there are other ways of doing it much better than we have 
today.

Writing to a file the way we do it is synchronized, anyway, so the goal 
was only to achieve non funky dates.
    PrintWriter.java
    public void println(String x) {
    synchronized (lock) {
        print(x);
        println();
    }
    }


Filip

sebb wrote:
> On 18/06/2009, sebb <sebbaz@gmail.com> wrote:
>   
>> On 18/06/2009, Mark Thomas <markt@apache.org> wrote:
>>  > Tim Funk wrote:
>>  >  > I think this needs to be volatile ?
>>  >  > In (GetDateBenchmarkTest)
>>  >  >> +        private long currentMillis = 0;
>>  >  >
>>  >  > Same for (in TimeDateElementBenchmarkTest)
>>  >  >> +        private Date currentDate = null;
>>  >  >
>>  >  > Of course - since the test is single threaded - it doesn't really matter.
>>  >
>>  >
>>  > The test should be multi-threaded unless I got something badly wrong. I'll
>>  >  double check.
>>  >
>>  >  Making those volatile gets them closer to the real code. I doubt it will make a
>>  >  difference but worth changing to be sure. You never know with these things.
>>
>>
>> The field GetDateBenchmarkTest.currentDate is set in a synch. block in
>>  doSynch(), but for the return it is fetched outside the synch. block -
>>  so it could potentially be changed by another thread. Also if the
>>  synch. block is not entered, the thread might not see the correct
>>  version of the field as there is no synch. on the read.
>>
>>  Similarly in TimeDateElementBenchmarkTest.getDateSync() - although the
>>  field is volatile, it is set in the synch. block but fetched for the
>>  return outside the block.
>>
>>  If it is intended to allow currentDate to be updated by another thread
>>  before the return, then the field needs to be volatile. Otherwise the
>>  return value needs to be established in the synch. block.
>>
>>     
>
> Oops, forgot to mention - there are only 5 threads in the test; it
> might be useful to try tests with increasing numbers of threads to see
> if the relative numbers change.
>
>   
>>  >  Mark
>>  >
>>  >
>>  >  >
>>  >  > -Tim
>>  >  >
>>  >  > markt@apache.org wrote:
>>  >  >> Author: markt
>>  >  >> Date: Thu Jun 18 08:32:29 2009
>>  >  >> New Revision: 785952
>>  >  >>
>>  >  >> URL: http://svn.apache.org/viewvc?rev=785952&view=rev
>>  >  >> Log:
>>  >  >> Add some micro-benchmarks that enable the differences between the Sync
>>  >  >> and ThreadLocal approach to be compared
>>  >  >
>>  >  >
>>  >  > ---------------------------------------------------------------------
>>  >  > To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>  >  > For additional commands, e-mail: dev-help@tomcat.apache.org
>>  >  >
>>  >
>>  >
>>  >  ---------------------------------------------------------------------
>>  >  To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>  >  For additional commands, e-mail: dev-help@tomcat.apache.org
>>  >
>>  >
>>
>>     
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>   


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