Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-19 Thread Michael McMahon

gustav trede wrote:
On 19 August 2010 17:50, Chris Hegarty > wrote:


gustav trede wrote:


On 18 August 2010 13:22, Michael McMahon
mailto:michael.x.mcma...@oracle.com>
>> wrote:

   gustav trede wrote:


   On 18 August 2010 12:10, Chris Hegarty
mailto:chris.hega...@oracle.com>
   >
   
   
 
 
 





   Why not use a threadlocal dateformater ?.

   perhaps ...

   For certain cases Its also viable to exploit the fact
that its
   enough to generate a new value once per second for HTTP
timestamps.
   Even if its not "needed", it would imo be nice if the
JDK code
   itself could somehow act as reference / good examples
of how to
   THINK(design) and implement.

   I suspect you're looking at this from a server perspective.
This
   code is involved with parsing
   of incoming cookies. So, the generation of timestamps isn't
being
   done here.

Yes i am quite server focused =), servers can act as http
clients too, different kind of intermediate logic etc.
Anyhow, my apologies for wasting your time with this not so
important issue, my brain just pings on code it finds strange /.


We use ThreadLocal to store the formatter for the HttpServer
implementation, com.sun.net.httpserver, see 6967684 "httpserver
using a non thread-safe SimpleDateFormat" [1]. The threads
creating the HTTP timestamps are part of an execurtorService and
will need a timestamp per response. These threads will not be used
for anything else, and are most probably going to handle many
requests. I think adding a threadLocal makes sense for these.

For HttpCookie, it is client side and a thread may only ever
handle a few cookies for its lifetime. I think adding the overhead
of three formatters may just be wasteful since the thread may
never do any more than a few HTTP requests.

Are you ok with this change?

Sounds ok to me, considering how the current overall design and public 
API is, the hands are a bit tied for radical changes.


regards
 gustav trede

Me too.

- Michael.


Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-19 Thread gustav trede
On 19 August 2010 17:50, Chris Hegarty  wrote:

> gustav trede wrote:
>
>>
>> On 18 August 2010 13:22, Michael McMahon 
>> > michael.x.mcma...@oracle.com>> wrote:
>>
>>gustav trede wrote:
>>
>>
>>On 18 August 2010 12:10, Chris Hegarty >
>>>>> wrote:
>>
>>   Michael,
>>
>>   java.net.HttpCookie uses static SimpleDateFormat which is not
>>   thread safe. I think the best solution here is to simply create
>>   local SimpleDateFormat as needed.
>>
>>   Webrev:
>>
>> http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/
>>
>>   > >
>>
>>
>>
>>Why not use a threadlocal dateformater ?.
>>
>>perhaps ...
>>
>>For certain cases Its also viable to exploit the fact that its
>>enough to generate a new value once per second for HTTP timestamps.
>>Even if its not "needed", it would imo be nice if the JDK code
>>itself could somehow act as reference / good examples of how to
>>THINK(design) and implement.
>>
>>I suspect you're looking at this from a server perspective. This
>>code is involved with parsing
>>of incoming cookies. So, the generation of timestamps isn't being
>>done here.
>>
>> Yes i am quite server focused =), servers can act as http clients too,
>> different kind of intermediate logic etc.
>> Anyhow, my apologies for wasting your time with this not so important
>> issue, my brain just pings on code it finds strange /.
>>
>
> We use ThreadLocal to store the formatter for the HttpServer
> implementation, com.sun.net.httpserver, see 6967684 "httpserver using a non
> thread-safe SimpleDateFormat" [1]. The threads creating the HTTP timestamps
> are part of an execurtorService and will need a timestamp per response.
> These threads will not be used for anything else, and are most probably
> going to handle many requests. I think adding a threadLocal makes sense for
> these.
>
> For HttpCookie, it is client side and a thread may only ever handle a few
> cookies for its lifetime. I think adding the overhead of three formatters
> may just be wasteful since the thread may never do any more than a few HTTP
> requests.
>
> Are you ok with this change?
>
> Sounds ok to me, considering how the current overall design and public API
is, the hands are a bit tied for radical changes.

regards
 gustav trede


Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-19 Thread Chris Hegarty

gustav trede wrote:


On 18 August 2010 13:22, Michael McMahon > wrote:


gustav trede wrote:


On 18 August 2010 12:10, Chris Hegarty mailto:chris.hega...@oracle.com>
>> wrote:

   Michael,

   java.net.HttpCookie uses static SimpleDateFormat which is not
   thread safe. I think the best solution here is to simply create
   local SimpleDateFormat as needed.

   Webrev:
http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/

   



Why not use a threadlocal dateformater ?.

perhaps ...

For certain cases Its also viable to exploit the fact that its
enough to generate a new value once per second for HTTP timestamps.
Even if its not "needed", it would imo be nice if the JDK code
itself could somehow act as reference / good examples of how to
THINK(design) and implement.

I suspect you're looking at this from a server perspective. This
code is involved with parsing
of incoming cookies. So, the generation of timestamps isn't being
done here.

Yes i am quite server focused =), servers can act as http clients too, 
different kind of intermediate logic etc.
Anyhow, my apologies for wasting your time with this not so important 
issue, my brain just pings on code it finds strange /.


We use ThreadLocal to store the formatter for the HttpServer 
implementation, com.sun.net.httpserver, see 6967684 "httpserver using a 
non thread-safe SimpleDateFormat" [1]. The threads creating the HTTP 
timestamps are part of an execurtorService and will need a timestamp per 
response. These threads will not be used for anything else, and are most 
probably going to handle many requests. I think adding a threadLocal 
makes sense for these.


For HttpCookie, it is client side and a thread may only ever handle a 
few cookies for its lifetime. I think adding the overhead of three 
formatters may just be wasteful since the thread may never do any more 
than a few HTTP requests.


Are you ok with this change?

-Chris.

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6967684




regards
  gustav trede


Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-18 Thread gustav trede
On 18 August 2010 13:22, Michael McMahon wrote:

> gustav trede wrote:
>
>>
>> On 18 August 2010 12:10, Chris Hegarty > chris.hega...@oracle.com>> wrote:
>>
>>Michael,
>>
>>java.net.HttpCookie uses static SimpleDateFormat which is not
>>thread safe. I think the best solution here is to simply create
>>local SimpleDateFormat as needed.
>>
>>Webrev:
>> 
>> http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/
>>
>>
>>
>>
>> Why not use a threadlocal dateformater ?.
>>
> perhaps ...
>
>  For certain cases Its also viable to exploit the fact that its enough to
>> generate a new value once per second for HTTP timestamps.
>> Even if its not "needed", it would imo be nice if the JDK code itself
>> could somehow act as reference / good examples of how to THINK(design) and
>> implement.
>>
>>  I suspect you're looking at this from a server perspective. This code is
> involved with parsing
> of incoming cookies. So, the generation of timestamps isn't being done
> here.
>
> Yes i am quite server focused =), servers can act as http clients too,
different kind of intermediate logic etc.
Anyhow, my apologies for wasting your time with this not so important issue,
my brain just pings on code it finds strange /.


regards
  gustav trede


Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-18 Thread Michael McMahon

gustav trede wrote:


On 18 August 2010 12:10, Chris Hegarty > wrote:


Michael,

java.net.HttpCookie uses static SimpleDateFormat which is not
thread safe. I think the best solution here is to simply create
local SimpleDateFormat as needed.

Webrev:
 http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/



Why not use a threadlocal dateformater ?.

perhaps ...
For certain cases Its also viable to exploit the fact that its enough 
to generate a new value once per second for HTTP timestamps.
Even if its not "needed", it would imo be nice if the JDK code itself 
could somehow act as reference / good examples of how to THINK(design) 
and implement.


I suspect you're looking at this from a server perspective. This code is 
involved with parsing

of incoming cookies. So, the generation of timestamps isn't being done here.

- Michael.


regards
   gustav trede




Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-18 Thread gustav trede
On 18 August 2010 12:47, Chris Hegarty  wrote:

> On 18/08/2010 11:28, gustav trede wrote:
>
>>
>> On 18 August 2010 12:10, Chris Hegarty > > wrote:
>>
>>Michael,
>>
>>java.net.HttpCookie uses static SimpleDateFormat which is not thread
>>safe. I think the best solution here is to simply create local
>>SimpleDateFormat as needed.
>>
>>Webrev:
>>
>> http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/
>>
>>
>>
>>
>> Why not use a threadlocal dateformater ?.
>>
>
> I guess it depends on the use of HttpCookie. In the JDK HttpCookie is only
> used to parse cookies sent in a HTTP response. For this type of application
> potentially keeping three formatters per thread seems like a waste. This, of
> course, is only one use.
>
>
>  For certain cases Its also viable to exploit the fact that its enough to
>> generate a new value once per second for HTTP timestamps.
>>
>
> I don't understand. Are you using HttpCookie in a server type context?
>
> No, i use my custom server code, but i do use the concept of generating
HTTP DATE:  header timestamp values once per second by updating the per
cached file/standard responses directbytebuffers using only volatile
somantics.


-Chris.
>
>
>  Even if its not "needed", it would imo be nice if the JDK code itself
>> could somehow act as reference / good examples of how to THINK(design)
>> and implement.
>>
>>
>> regards
>>gustav trede
>>
>


Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-18 Thread Chris Hegarty

On 18/08/2010 11:28, gustav trede wrote:


On 18 August 2010 12:10, Chris Hegarty mailto:chris.hega...@oracle.com>> wrote:

Michael,

java.net.HttpCookie uses static SimpleDateFormat which is not thread
safe. I think the best solution here is to simply create local
SimpleDateFormat as needed.

Webrev:
http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/



Why not use a threadlocal dateformater ?.


I guess it depends on the use of HttpCookie. In the JDK HttpCookie is 
only used to parse cookies sent in a HTTP response. For this type of 
application potentially keeping three formatters per thread seems like a 
waste. This, of course, is only one use.



For certain cases Its also viable to exploit the fact that its enough to
generate a new value once per second for HTTP timestamps.


I don't understand. Are you using HttpCookie in a server type context?

-Chris.


Even if its not "needed", it would imo be nice if the JDK code itself
could somehow act as reference / good examples of how to THINK(design)
and implement.


regards
gustav trede


Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

2010-08-18 Thread gustav trede
On 18 August 2010 12:10, Chris Hegarty  wrote:

> Michael,
>
> java.net.HttpCookie uses static SimpleDateFormat which is not thread safe.
> I think the best solution here is to simply create local SimpleDateFormat as
> needed.
>
> Webrev:
>  
> http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/
>
>
Why not use a threadlocal dateformater ?.
For certain cases Its also viable to exploit the fact that its enough to
generate a new value once per second for HTTP timestamps.
Even if its not "needed", it would imo be nice if the JDK code itself could
somehow act as reference / good examples of how to THINK(design) and
implement.


regards
   gustav trede