Apart from the unresolved issue of logging, I still think the commit should be reverted because it combines two completely different changes.
Please can you revert the commit ASAP? On 2 June 2014 18:09, sebb <seb...@gmail.com> wrote: > On 2 June 2014 16:11, Romain Manni-Bucau <rmannibu...@gmail.com> wrote: >> First about immutabilit thread safety etc: we can use final if it ends the >> topic, it was not cause first version was a field and not a constant and >> serializable but now it can be final. >> >> Then about isDebugEnabled: overhead is quite important compared to a simple >> boolean test. Most of the time it is not important but for a caching >> solution (in particular in memory mode) it is impacting since it is done >> very often. >> >> To be convinced of it just debug log4j (1.2) impl for instance. Really >> depends the config too but basically you'll end up checking repository >> level + potentially browse all logger categories. If config is well done no >> by overhead by if not that's really too much. If you take JUL that's worse. >> isDebugEnabled is fast but then log invocation has more check (record, >> filter, handlers at least). Actually I think we can do further proposing a >> JCS property "verbose" and get rid of logger level for these cases. We can >> add a shared MBean to on/off it then. >> >> >> wdyt? > > I think we need more proof that some kind of caching really is needed. > >> >> >> >> >> >> >> Romain Manni-Bucau >> Twitter: @rmannibucau >> Blog: http://rmannibucau.wordpress.com/ >> LinkedIn: http://fr.linkedin.com/in/rmannibucau >> Github: https://github.com/rmannibucau >> >> >> 2014-06-02 16:27 GMT+02:00 Phil Steitz <phil.ste...@gmail.com>: >> >>> On 6/1/14, 6:01 PM, Bernd Eckenfels wrote: >>> > Am Sun, 1 Jun 2014 23:43:10 +0100 >>> > schrieb sebb <seb...@gmail.com>: >>> > >>> >> On 1 June 2014 20:19, Romain Manni-Bucau <rmannibu...@gmail.com> >>> >> wrote: >>> >>> well it is for sure thread safe. Not sure I get why final and synch >>> >>> would be mandatory in this particular case (field will maybe be >>> >>> cached by thread but that's not an issue since the value will be >>> >>> unique). >>> >> non-final fields are not guaranteed to be published across threads in >>> >> the absence of sync. >>> > The two fields wont change, so there is no need for publishing changes. >>> > So they dont need to be volatile. They could be made however final to >>> > make it clearer that they will not change (but IMHO this does not make >>> > them more or less thread safe). >>> >>> Right, except that the logger is itself mutable and it looks like >>> clients hold onto references to it. What I don't get is why it is >>> so much faster to add the overhead of the helper just to avoid a >>> call to logger.isDebugEnabled(). I would expect that to return just >>> as fast as the LOG_HELPER inspecting its (even cached) boolean. >>> What am I missing? >>> >>> Phil >>> > >>> > I feel indifferent about beeing able to turn off trace/debug by >>> > overwriting the underlying logger. If we are really so logger >>> > agnostic it is probably a good idea. At least when commons-logging is >>> > not able to abstract this shortcoming away. >>> > >>> > Gruss >>> > Bernd >>> > >>> > --------------------------------------------------------------------- >>> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>> > For additional commands, e-mail: dev-h...@commons.apache.org >>> > >>> > >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >>> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
