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]> 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 >
