On 1 June 2014 18:54, Romain Manni-Bucau <rmannibu...@gmail.com> wrote: > > 2014-06-01 19:45 GMT+02:00 sebb <seb...@gmail.com>: >> >> PING >> > > Pong, sorry, missed this one. > >> >> On 29 May 2014 03:00, sebb <seb...@gmail.com> wrote: >> > On 28 May 2014 18:06, <rmannibu...@apache.org> wrote: >> >> Author: rmannibucau >> >> Date: Wed May 28 17:06:12 2014 >> >> New Revision: 1598071 >> >> >> >> URL: http://svn.apache.org/r1598071 >> >> Log: >> >> using reentrant locks instead of old synchronized >> > >> > -1 >> > >> > This commit mixes two completely different changes: >> > - re-entrant locks >> > - addition of LogHelper >> > >> > I think the LogHelper class is a bad idea. It is also not thread-safe. >> > If the cache is enabled, then it is not possible to change the logging >> > level during a run. >> > > > > How can it be not thread safe? (it is a constant)
The class is not thread-safe as it has a non-final non-synch variable > You can disable caching. The main point is jcs tests a lot log debug level > and depending your backing impl it can be slow (JUL, Log4j 1.2 for what I > tested). There is a property if you want to disable this feature but > actually in prod you rarely do it. > I have two objections to the commit. 1) the commit mixes two completely different changes 2) the LogHelper class is a bad idea, as it prevents changing the logging level. I suggest reverting the commit and re-applying the locking change on its own. The logging change really needs to be discussed first. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
