Looking at this some more, I now wonder why LoggerConfig.clearAppenders() even needs to wait for completion: LoggerConfig.clearAppenders() is only called from AbstractConfiguration.stop(), and is called *after* all appenders have been stopped...
Could we do away with the waitForCompletion() method (and associated fields) altogether, or am I missing something? On Sunday, September 13, 2015, Remko Popma <[email protected]> wrote: > Ralph, yes, pull up the shutdown.get() check into afterLogEvent(): > > > Gary, I expect some improvement, although it may not be huge. It should > also be visible in a single-threaded case. I hope small changes like this > allow us to catch up or exceed Logback in synchronous logging scenarios. > > On Sunday, September 13, 2015, Gary Gregory <[email protected] > <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: > >> And this will make logging faster because there is less locking in >> multi-threaded apps? >> >> Gary >> >> On Sat, Sep 12, 2015 at 1:10 PM, Ralph Goers <[email protected]> >> wrote: >> >>> Are you talking about making this change in >>> signalCompletionIfShutdown()? If so I am OK with what you are proposing. >>> >>> Ralph >>> >>> > On Sep 12, 2015, at 11:19 AM, Remko Popma <[email protected]> >>> wrote: >>> > >>> > I want to run this by you: >>> > >>> > LoggerConfig currently always takes a lock after logging a log event. >>> > Within this lock it checks if the shutdown flag has been set. >>> > Can we do it the other way around? First check if the shutdown flag >>> has been set and then take the lock (to notify the other thread)? >>> > >>> > I think it is safe to do this: >>> > >>> > public void log(final LogEvent event) { >>> > counter.incrementAndGet(); >>> > try { >>> > if (!isFiltered(event)) { >>> > processLogEvent(event); >>> > } >>> > } finally { >>> > // FIRST update counter, THEN check shutdown flag; >>> > // these are atomic and will not get re-ordered. >>> > // So we don't need the lock to check the shutdown flag. >>> > if (counter.decrementAndGet() == 0 && shutdown.get()) { >>> > // ONLY take the lock if the shutdown flag was set >>> > shutdownLock.lock(); >>> > try { >>> > noLogEvents.signalAll(); // lock required to call >>> signal() >>> > } finally { >>> > shutdownLock.unlock(); >>> > } >>> > } >>> > } >>> > } >>> > >>> > // keep this method unchanged >>> > private void waitForCompletion() { >>> > shutdownLock.lock(); >>> > try { >>> > // NOTE: FIRST the shutdown flag is set >>> > if (shutdown.compareAndSet(false, true)) { >>> > int retries = 0; >>> > // NOTE: THEN the counter is read >>> > while (counter.get() > 0) { >>> > try { >>> > noLogEvents.await(retries + 1, TimeUnit.SECONDS); >>> > } catch (final InterruptedException ie) { >>> > if (++retries > MAX_RETRIES) { >>> > break; >>> > } >>> > } >>> > } >>> > } >>> > } finally { >>> > shutdownLock.unlock(); >>> > } >>> > } >>> > >>> > >>> >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: [email protected] >>> For additional commands, e-mail: [email protected] >>> >>> >> >> >> -- >> E-Mail: [email protected] | [email protected] >> Java Persistence with Hibernate, Second Edition >> <http://www.manning.com/bauer3/> >> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >> Spring Batch in Action <http://www.manning.com/templier/> >> Blog: http://garygregory.wordpress.com >> Home: http://garygregory.com/ >> Tweet! http://twitter.com/GaryGregory >> >
