Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe
gustav trede wrote: On 18 August 2010 13:22, Michael McMahon michael.x.mcma...@oracle.com mailto:michael.x.mcma...@oracle.com wrote: gustav trede wrote: On 18 August 2010 12:10, Chris Hegarty chris.hega...@oracle.com mailto:chris.hega...@oracle.com mailto:chris.hega...@oracle.com 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/ http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/ http://cr.openjdk.java.net/%7Echegar/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
On 19 August 2010 17:50, Chris Hegarty chris.hega...@oracle.com wrote: gustav trede wrote: On 18 August 2010 13:22, Michael McMahon michael.x.mcma...@oracle.commailto: michael.x.mcma...@oracle.com wrote: gustav trede wrote: On 18 August 2010 12:10, Chris Hegarty chris.hega...@oracle.com mailto:chris.hega...@oracle.com mailto:chris.hega...@oracle.com 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/http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/ http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/ http://cr.openjdk.java.net/%7Echegar/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
gustav trede wrote: On 19 August 2010 17:50, Chris Hegarty chris.hega...@oracle.com mailto:chris.hega...@oracle.com wrote: gustav trede wrote: On 18 August 2010 13:22, Michael McMahon michael.x.mcma...@oracle.com mailto:michael.x.mcma...@oracle.com mailto:michael.x.mcma...@oracle.com mailto:michael.x.mcma...@oracle.com wrote: gustav trede wrote: On 18 August 2010 12:10, Chris Hegarty chris.hega...@oracle.com mailto:chris.hega...@oracle.com mailto:chris.hega...@oracle.com mailto:chris.hega...@oracle.com mailto:chris.hega...@oracle.com mailto:chris.hega...@oracle.com mailto:chris.hega...@oracle.com 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/ http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/ http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/ http://cr.openjdk.java.net/%7Echegar/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 Me too. - Michael.
Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe
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/ Thanks, -Chris.
Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe
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/http://cr.openjdk.java.net/%7Echegar/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
Re: Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe
On 18/08/2010 11:28, gustav trede wrote: On 18 August 2010 12:10, Chris Hegarty chris.hega...@oracle.com 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/ http://cr.openjdk.java.net/%7Echegar/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
On 18 August 2010 12:47, Chris Hegarty chris.hega...@oracle.com wrote: On 18/08/2010 11:28, gustav trede wrote: On 18 August 2010 12:10, Chris Hegarty chris.hega...@oracle.com 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/http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/ http://cr.openjdk.java.net/%7Echegar/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
gustav trede wrote: On 18 August 2010 12:10, Chris Hegarty chris.hega...@oracle.com 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/ http://cr.openjdk.java.net/%7Echegar/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
On 18 August 2010 13:22, Michael McMahon michael.x.mcma...@oracle.comwrote: gustav trede wrote: On 18 August 2010 12:10, Chris Hegarty chris.hega...@oracle.com 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/http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/ http://cr.openjdk.java.net/%7Echegar/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
