The problem is, lock’s don’t “close” so they shouldn’t be Autoacloseable at all.
Ralph > On Jul 21, 2016, at 5:12 PM, Matt Sicker <boa...@gmail.com> wrote: > > AutoCloseable versus AutoCloseableLock. > > On 21 July 2016 at 18:19, Gary Gregory <garydgreg...@gmail.com > <mailto:garydgreg...@gmail.com>> wrote: > On Thu, Jul 21, 2016 at 4:12 PM, Matt Sicker <boa...@gmail.com > <mailto: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 > <mailto: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 > <mailto: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 > <mailto: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 >> <mailto: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 >> <mailto:boa...@gmail.com>> wrote: >> I kinda imagined AutoCloseableLock to implement both AutoCloseable and Lock. >> >> ---------- Forwarded message ---------- >> From: <ggreg...@apache.org <mailto:ggreg...@apache.org>> >> Date: 24 June 2016 at 03:50 >> Subject: [1/4] logging-log4j2 git commit: Add AutoCloseableLock. >> To: comm...@logging.apache.org <mailto: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 >> <http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo> >> Commit: >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/40efa80a >> <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 >> <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 >> <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 <mailto:ggreg...@apache.org>> >> Authored: Thu Jun 23 21:59:02 2016 -0700 >> Committer: Gary Gregory <ggreg...@apache.org <mailto: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 >> >> <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 >> <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 <mailto:boa...@gmail.com>> >> >> >> >> -- >> E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | >> ggreg...@apache.org <mailto: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 <http://garygregory.wordpress.com/> >> Home: http://garygregory.com/ <http://garygregory.com/> >> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> > > > > -- > Matt Sicker <boa...@gmail.com <mailto:boa...@gmail.com>> > > > > -- > E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | > ggreg...@apache.org <mailto: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 <http://garygregory.wordpress.com/> > Home: http://garygregory.com/ <http://garygregory.com/> > Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> > > > -- > Matt Sicker <boa...@gmail.com <mailto:boa...@gmail.com>> > > > > -- > E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | > ggreg...@apache.org <mailto: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 <http://garygregory.wordpress.com/> > Home: http://garygregory.com/ <http://garygregory.com/> > Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> > > > -- > Matt Sicker <boa...@gmail.com <mailto:boa...@gmail.com>>
