Mostly LGTM.  This looks like a nice cleanup and should remove a lot of
potential for lock contention.  I like that a lot of the code gets a lot
simpler too.  SerializabilityUtil.java is some particularly embarrassing
code that I helped write.  This patch should allow that code to be
refactored to be simpler in the future, I'd think.



http://gwt-code-reviews.appspot.com/716801/diff/14001/15001
File dev/core/src/com/google/gwt/dev/util/HttpHeaders.java (right):

http://gwt-code-reviews.appspot.com/716801/diff/14001/15001#newcode65
dev/core/src/com/google/gwt/dev/util/HttpHeaders.java:65: * The Internet
date format for HTTP.
It would be great to add a comment here as to why we have to go through
all this trouble in the first place.  Just something like "The doc for
DataFormat states: Date formats are not synchronized. It is recommended
to create separate format instances for each thread. If multiple threads
access a format concurrently, it must be synchronized externally."

http://gwt-code-reviews.appspot.com/716801/diff/14001/15004
File
user/src/com/google/gwt/user/server/rpc/AbstractRemoteServiceServlet.java
(right):

http://gwt-code-reviews.appspot.com/716801/diff/14001/15004#newcode33
user/src/com/google/gwt/user/server/rpc/AbstractRemoteServiceServlet.java:33:
protected static final transient ThreadLocal<HttpServletRequest>
perThreadRequest =
Remove the "transient" keyword here and below.

http://gwt-code-reviews.appspot.com/716801/diff/14001/15007
File
user/src/com/google/gwt/user/server/rpc/impl/ConcurrentReferenceHashMap.java
(right):

http://gwt-code-reviews.appspot.com/716801/diff/14001/15007#newcode1025
user/src/com/google/gwt/user/server/rpc/impl/ConcurrentReferenceHashMap.java:1025:
public ConcurrentReferenceHashMap(int initialCapacity, boolean userefid)
{
We need to be really clear about where this class came from, what
version, what license it's under, etc.  Are we sure we can just rebrand
it as Apache?  Is there any reason to move it from the jsr166y package
to our own?  Did we modify this?  For example, I cannot find this
particular constructor in the public versions (or any constructor
overloads that take a boolean argument).

http://gwt-code-reviews.appspot.com/716801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to