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

Reply via email to