On Thu, Jul 21, 2016 at 4:12 PM, Matt Sicker <boa...@gmail.com> wrote:
> Well in that case, I guess it doesn't introduce garbage. I'm back to > supporting this, then. > > Can't you just do: > > try (final AutoCloseable a = configLock.lock()) { > // ... > } > That's just the example I gave! Am I missing something? ;-) Gary > > On 21 July 2016 at 18:10, Gary Gregory <garydgreg...@gmail.com> wrote: > >> Hi All, >> >> What extra garbage? (Putting aside if this a "good" idea or not.) A local >> variable here does not create an extra object to be GC'd. >> >> For example, in LoggerContext, we have: >> >> private final Lock configLock = new ReentrantLock(); >> ... >> >> @Override >> public void stop() { >> ... >> configLock.lock(); >> try { >> ... >> } finally { >> configLock.unlock(); >> } >> ... >> } >> >> >> Which I propose to replace with: >> >> private final AutoCloseableLock configLock = >> AutoCloseableLock.forReentrantLock(); >> ... >> >> @Override >> public void stop() { >> ... >> try (final AutoCloseableLock l = configLock.lock()) { >> ... >> } >> ... >> } >> >> Yes, AutoCloseableLock is an extra object on top of the Lock itself, I'll >> give you that of course. >> >> The "l" lvar is what the Java syntax requires but there is no extra >> object created by configLock.lock(), which returns "this". >> >> Gary >> >> >> On Thu, Jul 21, 2016 at 3:12 PM, Matt Sicker <boa...@gmail.com> wrote: >> >>> Yeah, adding garbage does seem like a bad idea in this case. >>> >>> On 21 July 2016 at 16:30, Ralph Goers <ralph.go...@dslextreme.com> >>> wrote: >>> >>>> The more I think about this the more I dislike it. This requires that a >>>> temporary variable be created for no other reason than to satisfy the >>>> compiler. It defeats the work Remko has been doing to make the code garbage >>>> free as it is explicitly creating objects it doesn’t even use. >>>> >>>> Ralph >>>> >>>> On Jul 21, 2016, at 2:12 PM, Gary Gregory <garydgreg...@gmail.com> >>>> wrote: >>>> >>>> Hi Matt, >>>> >>>> AutoCloseableLock cannot implement Lock because lock() is void and I >>>> count on AutoCloseableLock#lock() returning "this" for the pattern: >>>> >>>> try (final AutoCloseableLock l = LOCK.lock()) { >>>> return MAP.containsKey(name); >>>> } >>>> >>>> I could rename lock() to doLock() and a void lock() but that seems a >>>> bit confusing to have both methods. >>>> >>>> This is in the branch AutoCloseableLock which I'd like to merge. >>>> >>>> Thoughts? >>>> >>>> Gary >>>> >>>> On Fri, Jun 24, 2016 at 7:42 AM, Matt Sicker <boa...@gmail.com> wrote: >>>> >>>>> I kinda imagined AutoCloseableLock to implement both AutoCloseable and >>>>> Lock. >>>>> >>>>> ---------- Forwarded message ---------- >>>>> From: <ggreg...@apache.org> >>>>> Date: 24 June 2016 at 03:50 >>>>> Subject: [1/4] logging-log4j2 git commit: Add AutoCloseableLock. >>>>> To: comm...@logging.apache.org >>>>> >>>>> >>>>> Repository: logging-log4j2 >>>>> Updated Branches: >>>>> refs/heads/AutoCloseableLock [created] 72d9978c6 >>>>> >>>>> >>>>> Add AutoCloseableLock. >>>>> >>>>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >>>>> Commit: >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/40efa80a >>>>> Tree: >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/40efa80a >>>>> Diff: >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/40efa80a >>>>> >>>>> Branch: refs/heads/AutoCloseableLock >>>>> Commit: 40efa80a1a9745d7f9162b4f7ce96a7571a1c336 >>>>> Parents: bc296c5 >>>>> Author: Gary Gregory <ggreg...@apache.org> >>>>> Authored: Thu Jun 23 21:59:02 2016 -0700 >>>>> Committer: Gary Gregory <ggreg...@apache.org> >>>>> Committed: Thu Jun 23 21:59:02 2016 -0700 >>>>> >>>>> ---------------------------------------------------------------------- >>>>> .../logging/log4j/util/AutoCloseableLock.java | 44 >>>>> ++++++++++++++++++++ >>>>> 1 file changed, 44 insertions(+) >>>>> ---------------------------------------------------------------------- >>>>> >>>>> >>>>> >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/40efa80a/log4j-api/src/main/java/org/apache/logging/log4j/util/AutoCloseableLock.java >>>>> ---------------------------------------------------------------------- >>>>> diff --git >>>>> a/log4j-api/src/main/java/org/apache/logging/log4j/util/AutoCloseableLock.java >>>>> b/log4j-api/src/main/java/org/apache/logging/log4j/util/AutoCloseableLock.java >>>>> new file mode 100644 >>>>> index 0000000..65aa581 >>>>> --- /dev/null >>>>> +++ >>>>> b/log4j-api/src/main/java/org/apache/logging/log4j/util/AutoCloseableLock.java >>>>> @@ -0,0 +1,44 @@ >>>>> +/* >>>>> + * Licensed to the Apache Software Foundation (ASF) under one or more >>>>> + * contributor license agreements. See the NOTICE file distributed >>>>> with >>>>> + * this work for additional information regarding copyright ownership. >>>>> + * The ASF licenses this file to You under the Apache license, >>>>> Version 2.0 >>>>> + * (the "License"); you may not use this file except in compliance >>>>> with >>>>> + * the License. You may obtain a copy of the License at >>>>> + * >>>>> + * http://www.apache.org/licenses/LICENSE-2.0 >>>>> + * >>>>> + * Unless required by applicable law or agreed to in writing, software >>>>> + * distributed under the License is distributed on an "AS IS" BASIS, >>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >>>>> implied. >>>>> + * See the license for the specific language governing permissions and >>>>> + * limitations under the license. >>>>> + */ >>>>> +package org.apache.logging.log4j.util; >>>>> + >>>>> +import java.util.Objects; >>>>> +import java.util.concurrent.locks.Lock; >>>>> + >>>>> +public class AutoCloseableLock implements AutoCloseable { >>>>> + public static AutoCloseableLock lock(final Lock lock) { >>>>> + Objects.requireNonNull(lock, "lock"); >>>>> + lock.lock(); >>>>> + return new AutoCloseableLock(lock); >>>>> + } >>>>> + >>>>> + private final Lock lock; >>>>> + >>>>> + public AutoCloseableLock(final Lock lock) { >>>>> + this.lock = lock; >>>>> + } >>>>> + >>>>> + @Override >>>>> + public void close() { >>>>> + this.lock.unlock(); >>>>> + } >>>>> + >>>>> + public AutoCloseableLock lock() { >>>>> + this.lock.lock(); >>>>> + return this; >>>>> + } >>>>> +} >>>>> \ No newline at end of file >>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Matt Sicker <boa...@gmail.com> >>>>> >>>> >>>> >>>> >>>> -- >>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >>>> 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 >>>> >>>> >>>> >>> >>> >>> -- >>> Matt Sicker <boa...@gmail.com> >>> >> >> >> >> -- >> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >> 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 >> > > > > -- > Matt Sicker <boa...@gmail.com> > -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org 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