I can see your POV. It's just so "tempting" to use such a convenience!
Gary
On Thu, Jul 21, 2016 at 4:48 PM, Ralph Goers <ralph.go...@dslextreme.com>
wrote:
> You are correct, it isn’t creating extra garbage. However, I still object
> because close() is in no way equivalent to unlock().
>
> Ralph
>
> On Jul 21, 2016, at 4:10 PM, 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
>
>
>
--
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
