AutoCloseable versus AutoCloseableLock.
On 21 July 2016 at 18:19, Gary Gregory <garydgreg...@gmail.com> wrote:
> 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
>
--
Matt Sicker <boa...@gmail.com>
