Re: svn commit: r1476359 - in /subversion/trunk/subversion/bindings/javahl/src/org: apache/subversion/javahl/types/LogDate.java tigris/subversion/javahl/LogDate.java

On Mon, Apr 29, 2013 at 11:39 AM, Philip Martin
<philip.martin_at_wandisco.com> wrote:
> Mark Phippard <markphip_at_gmail.com> writes:
>
>> I think Synchronized is a better solution, but will not object if you
>> want to make the change.
>
> I'm a novice as far as Java is concerned, I don't know which style is
> preferred.

This is a decent resource that shows the different options:

http://www.javacodegeeks.com/2010/07/java-best-practices-dateformat-in.html

We could adopt the ThreadLocal option but as you point out this code
is not used a lot. So it seems like the simpler approach is better.
Besides the fact that the benchmarks that use multiple threads show
that Synchronized is faster than creating a new object, the reason I
like it is that it calls out the fact that there is a thread safety
issue for people to be aware of. If we implemented the other
approach, we could obviously add comments explaining why, but I think
it would still be prone to someone "optimizing" the code back to the
current state.

If someone really wants to max out the performance we could go the
ThreadLocal route. I think we should have more tests of this class if
we do that, since the code becomes more complicated. I did not want to
commit to writing those.

Anyway, as I said, based on the research you did (thanks!) I now
realize I can fix the problem in Subclipse where we were probably
"abusing" our usage of this class. The change I committed is still a
valid improvement, I just do not think there is a lot of reason to
optimize this any further.

--
Thanks
Mark Phippard
http://markphip.blogspot.com/