You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2020/06/26 14:06:51 UTC
[lang] org.apache.commons.lang3.concurrent.Locks.Lock
Hi All:
I know email is a challenging medium for code reviews, so please consider
these comments coming from my best intentions, constructive and caring ;-)
Also please excuse the meandering nature of this post.
The new class org.apache.commons.lang3.concurrent.Locks.Lock needs better
names IMO, not just the class name. There is already a JRE interface called
Lock so this class name is confusing (to me.)
The Javadoc reads in part:
* This class implements a wrapper for a locked (hidden) object, and
* provides the means to access it. The basic idea, is that the user
* code forsakes all references to the locked object, using only the
* wrapper object, and the accessor methods
But this is not the case, the object itself is not locked in
the traditional sense with a monitor through a synchronized block, so we
need to update the Javadoc as well.
To me, this is more about executing blocks of code (through lambdas) which
then get 'safe' (via locking) access to an underlying object. This tells me
the class is more about the functional interfaces than about a domain
"Object", hence the name "Lock" is not the best. It's really more of a
visitor where the visitation pattern is: Lock, Visit, Unlock.
Instead, maybe:
- StampledLockVisitor (more specific)
- LockingVisitor (more general)
- SafeObject (vague)
- ObjectLocker (vague)
- rejected: LockedObject since the object itself is not locked.
- ?
What is also confusing IMO is that the instance variable for the object is
called "lockedObject" but the object is in fact NOT locked all the time. So
that needs to be renamed IMO:
- object (the simplest)
- subject
- domain
- target
- ?
In the same vein, the StampedLock is named "lock" which is also confusing
since StampedLock does not implement Lock.
Why can't the domain object be null, it's never used by the framework?
Why this:
if (t == lockedObject) {
throw new IllegalStateException("The returned object
is, in fact, the hidden object.");
}
?
This seems like an application specific constraint that has nothing to do
with the framework.
Now that I've considered the above, the API Locks.lock(O) is really
misnamed, because it does not lock anything, it's a factory method.
Stepping back even more, since there is only a static inner class in Locks,
and no-hint that alternative implementations for different kind of locks
are possible, I would say we do not need Locks, all we need is what is now
called Lock.
It's not clear why some methods are protected, I would make those private.
It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would then
make the current Lock class a standalone class which can then be used later
from a factory class (now Locks) where we can then fill in with different
implementations.
The Javadoc talks about a "hidden" object but it is not hidden since it is
passed to all visitors!
This test assumption is wrong:
If our threads are running concurrently, then we expect to be
faster
than running one after the other.
The VM or Java spec makes no such guarantee and the tests have no control
over VM scheduling. There are cases where this will fail when under heavy
load for example where the cost of additional threads becomes overwhelming.
Another item I am quite doubtful about is hiding checked exceptions by
rethrowning them as unchecked. I'd consider this an anti-pattern. If I am
using one of our "Failing" interfaces it is because I am expecting it to
fail. Perhaps this could be made smoother by refactoring to pass in a
lambda for an exception handler.
I've crystallized my thoughts into code here as WIP (Javadoc needs work):
https://github.com/apache/commons-lang/pull/559
Not as important as the above:
The example in the Javadoc uses logging as its domain subject, a "logging"
API (PrintStream) which is not a good example IMO. Logging frameworks
today like Log4j handle multi-threaded applications normally without having
developers meddle in it. Yes, I understand it's a simple example but I am
hoping we can come up with something more realistic or useful.
Thank you,
Gary
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Jul 7, 2020 at 10:22 PM Gary Gregory <ga...@gmail.com> wrote:
> On Tue, Jul 7, 2020 at 7:14 PM Matt Sicker <bo...@gmail.com> wrote:
>
>> JCIP seems to call this idea a monitor, but that’s also the general
>> implicit locking mechanism in Java.
>>
>
FTR, that's JCIP section 4.2.1 (in my edition).
Gary
>
> Hi Matt,
>
> Right, and it is tempting to rename LockingVisitors.AbstractLockVisitor's
> 'object' ivar to 'monitor' but that is somewhat misleading because it is
> not that object that you are locking, instead you are locking an actual
> lock and then visiting that object. So I am inclined to keep that part but
> Monitor could be in the class name somewhere.
>
> Gary
>
>
>> On Tue, Jul 7, 2020 at 17:56 Gary Gregory <ga...@gmail.com> wrote:
>>
>> > Typo: LockingVisitors.
>> >
>> > On Tue, Jul 7, 2020 at 6:56 PM Gary Gregory <ga...@gmail.com>
>> > wrote:
>> >
>> > > In the PR https://github.com/apache/commons-lang/pull/559 I am going
>> > > with "LockingVistors".
>> > >
>> > > Gary
>> > >
>> > > On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com>
>> wrote:
>> > >
>> > >> I’m not all that familiar with that area of academia. Let me see
>> what I
>> > >> can dig up
>> > >>
>> > >> -Rob
>> > >>
>> > >> > On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
>> > >> >
>> > >> > Are there any academic references for this? Or even something from
>> > >> > Java Concurrency in Practice? Those are good sources for names
>> around
>> > >> > concurrency.
>> > >> >
>> > >> > On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com>
>> wrote:
>> > >> >>
>> > >> >> Why not make the name “ThreadedLambdaSynchronizer” or something
>> like
>> > >> that….just something more descriptive here that get’s closer to what
>> the
>> > >> intended functionality is.
>> > >> >>
>> > >> >> That said, I’m not particularly tied to the name
>> > >> “ThreadedLambdaSynchronizer” just spitballing here for something more
>> > >> descriptive than “Lock”
>> > >> >>
>> > >> >> -Rob
>> > >> >>
>> > >> >>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com>
>> wrote:
>> > >> >>>
>> > >> >>> Now that starts to sound like Apache Groovy or Kotlin.
>> > >> >>>
>> > >> >>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com>
>> > wrote:
>> > >> >>>>
>> > >> >>>> soemtimes I really wish to rewrite/add some functions in jdk
>> > >> directly...
>> > >> >>>> especially for reusing some package private static functions...
>> > >> >>>>
>> > >> >>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
>> > >> >>>>
>> > >> >>>>> I'm not sure talking to the JDK folks is helpful IMO. We are
>> still
>> > >> >>>>> targeting Java 8. The customers I deal with are migrating from
>> 8
>> > to
>> > >> 11, and
>> > >> >>>>> Java 11 is not everywhere our customers are. So talking about
>> > >> something
>> > >> >>>>> that might end up in Java... 25 seems to be not in our user's
>> best
>> > >> or
>> > >> >>>>> immediate interest. It might be good for Java in the long oh so
>> > >> long term
>> > >> >>>>> of course.
>> > >> >>>>>
>> > >> >>>>> Gary
>> > >> >>>>>
>> > >> >>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <
>> chtompki@gmail.com>
>> > >> wrote:
>> > >> >>>>>
>> > >> >>>>>> At first look, I’m a little surprised we’re trying to take on
>> > this
>> > >> >>>>>> functionality. Has anyone reached out to the JDK guys to see
>> if
>> > >> they’d be
>> > >> >>>>>> interested in having it in the JDK? That said, if we approach
>> it
>> > >> from
>> > >> >>>>> that
>> > >> >>>>>> path, we would lose the functionality in older versions of
>> java.
>> > So
>> > >> >>>>> maybe I
>> > >> >>>>>> just talked myself out of the idea of putting it in the
>> JDK....
>> > >> >>>>>>
>> > >> >>>>>> Just wanted to stream of consciousness my initial gut vibe.
>> > >> >>>>>>
>> > >> >>>>>> Cheers,
>> > >> >>>>>> -Rob
>> > >> >>>>>>
>> > >> >>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <
>> > >> garydgregory@gmail.com>
>> > >> >>>>>> wrote:
>> > >> >>>>>>>
>> > >> >>>>>>> Hi All:
>> > >> >>>>>>>
>> > >> >>>>>>> I know email is a challenging medium for code reviews, so
>> please
>> > >> >>>>> consider
>> > >> >>>>>>> these comments coming from my best intentions, constructive
>> and
>> > >> caring
>> > >> >>>>>> ;-)
>> > >> >>>>>>> Also please excuse the meandering nature of this post.
>> > >> >>>>>>>
>> > >> >>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock
>> > needs
>> > >> >>>>> better
>> > >> >>>>>>> names IMO, not just the class name. There is already a JRE
>> > >> interface
>> > >> >>>>>> called
>> > >> >>>>>>> Lock so this class name is confusing (to me.)
>> > >> >>>>>>>
>> > >> >>>>>>> The Javadoc reads in part:
>> > >> >>>>>>>
>> > >> >>>>>>> * This class implements a wrapper for a locked (hidden)
>> > object,
>> > >> and
>> > >> >>>>>>> * provides the means to access it. The basic idea, is that
>> the
>> > >> user
>> > >> >>>>>>> * code forsakes all references to the locked object, using
>> > only
>> > >> the
>> > >> >>>>>>> * wrapper object, and the accessor methods
>> > >> >>>>>>>
>> > >> >>>>>>> But this is not the case, the object itself is not locked in
>> > >> >>>>>>> the traditional sense with a monitor through a synchronized
>> > >> block, so
>> > >> >>>>> we
>> > >> >>>>>>> need to update the Javadoc as well.
>> > >> >>>>>>>
>> > >> >>>>>>> To me, this is more about executing blocks of code (through
>> > >> lambdas)
>> > >> >>>>>> which
>> > >> >>>>>>> then get 'safe' (via locking) access to an underlying object.
>> > This
>> > >> >>>>> tells
>> > >> >>>>>> me
>> > >> >>>>>>> the class is more about the functional interfaces than about
>> a
>> > >> domain
>> > >> >>>>>>> "Object", hence the name "Lock" is not the best. It's really
>> > more
>> > >> of a
>> > >> >>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
>> > >> >>>>>>> Instead, maybe:
>> > >> >>>>>>> - StampledLockVisitor (more specific)
>> > >> >>>>>>> - LockingVisitor (more general)
>> > >> >>>>>>> - SafeObject (vague)
>> > >> >>>>>>> - ObjectLocker (vague)
>> > >> >>>>>>> - rejected: LockedObject since the object itself is not
>> locked.
>> > >> >>>>>>> - ?
>> > >> >>>>>>>
>> > >> >>>>>>> What is also confusing IMO is that the instance variable for
>> the
>> > >> object
>> > >> >>>>>> is
>> > >> >>>>>>> called "lockedObject" but the object is in fact NOT locked
>> all
>> > the
>> > >> >>>>> time.
>> > >> >>>>>> So
>> > >> >>>>>>> that needs to be renamed IMO:
>> > >> >>>>>>> - object (the simplest)
>> > >> >>>>>>> - subject
>> > >> >>>>>>> - domain
>> > >> >>>>>>> - target
>> > >> >>>>>>> - ?
>> > >> >>>>>>>
>> > >> >>>>>>> In the same vein, the StampedLock is named "lock" which is
>> also
>> > >> >>>>> confusing
>> > >> >>>>>>> since StampedLock does not implement Lock.
>> > >> >>>>>>>
>> > >> >>>>>>> Why can't the domain object be null, it's never used by the
>> > >> framework?
>> > >> >>>>>>> Why this:
>> > >> >>>>>>> if (t == lockedObject) {
>> > >> >>>>>>> throw new IllegalStateException("The
>> returned
>> > >> object
>> > >> >>>>>>> is, in fact, the hidden object.");
>> > >> >>>>>>> }
>> > >> >>>>>>> ?
>> > >> >>>>>>> This seems like an application specific constraint that has
>> > >> nothing to
>> > >> >>>>> do
>> > >> >>>>>>> with the framework.
>> > >> >>>>>>>
>> > >> >>>>>>> Now that I've considered the above, the API Locks.lock(O) is
>> > >> really
>> > >> >>>>>>> misnamed, because it does not lock anything, it's a factory
>> > >> method.
>> > >> >>>>>>>
>> > >> >>>>>>> Stepping back even more, since there is only a static inner
>> > class
>> > >> in
>> > >> >>>>>> Locks,
>> > >> >>>>>>> and no-hint that alternative implementations for different
>> kind
>> > of
>> > >> >>>>> locks
>> > >> >>>>>>> are possible, I would say we do not need Locks, all we need
>> is
>> > >> what is
>> > >> >>>>>> now
>> > >> >>>>>>> called Lock.
>> > >> >>>>>>>
>> > >> >>>>>>> It's not clear why some methods are protected, I would make
>> > those
>> > >> >>>>>> private.
>> > >> >>>>>>> It's not like I can use or plugin a Lock,
>> > ReentrantReadWriteLock,
>> > >> a
>> > >> >>>>>>> ReentrantLock, or any ReadWriteLock instead of a
>> StampedLock. I
>> > >> would
>> > >> >>>>>> then
>> > >> >>>>>>> make the current Lock class a standalone class which can
>> then be
>> > >> used
>> > >> >>>>>> later
>> > >> >>>>>>> from a factory class (now Locks) where we can then fill in
>> with
>> > >> >>>>> different
>> > >> >>>>>>> implementations.
>> > >> >>>>>>>
>> > >> >>>>>>> The Javadoc talks about a "hidden" object but it is not
>> hidden
>> > >> since it
>> > >> >>>>>> is
>> > >> >>>>>>> passed to all visitors!
>> > >> >>>>>>>
>> > >> >>>>>>> This test assumption is wrong:
>> > >> >>>>>>>
>> > >> >>>>>>> If our threads are running concurrently, then we
>> expect to
>> > >> be
>> > >> >>>>>>> faster
>> > >> >>>>>>> than running one after the other.
>> > >> >>>>>>>
>> > >> >>>>>>> The VM or Java spec makes no such guarantee and the tests
>> have
>> > no
>> > >> >>>>> control
>> > >> >>>>>>> over VM scheduling. There are cases where this will fail when
>> > >> under
>> > >> >>>>> heavy
>> > >> >>>>>>> load for example where the cost of additional threads becomes
>> > >> >>>>>> overwhelming.
>> > >> >>>>>>>
>> > >> >>>>>>> Another item I am quite doubtful about is hiding checked
>> > >> exceptions by
>> > >> >>>>>>> rethrowning them as unchecked. I'd consider this an
>> > anti-pattern.
>> > >> If I
>> > >> >>>>> am
>> > >> >>>>>>> using one of our "Failing" interfaces it is because I am
>> > >> expecting it
>> > >> >>>>> to
>> > >> >>>>>>> fail. Perhaps this could be made smoother by refactoring to
>> pass
>> > >> in a
>> > >> >>>>>>> lambda for an exception handler.
>> > >> >>>>>>>
>> > >> >>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc
>> > needs
>> > >> >>>>> work):
>> > >> >>>>>>> https://github.com/apache/commons-lang/pull/559
>> > >> >>>>>>>
>> > >> >>>>>>> Not as important as the above:
>> > >> >>>>>>> The example in the Javadoc uses logging as its domain
>> subject, a
>> > >> >>>>>> "logging"
>> > >> >>>>>>> API (PrintStream) which is not a good example IMO. Logging
>> > >> frameworks
>> > >> >>>>>>> today like Log4j handle multi-threaded applications normally
>> > >> without
>> > >> >>>>>> having
>> > >> >>>>>>> developers meddle in it. Yes, I understand it's a simple
>> example
>> > >> but I
>> > >> >>>>> am
>> > >> >>>>>>> hoping we can come up with something more realistic or
>> useful.
>> > >> >>>>>>>
>> > >> >>>>>>> Thank you,
>> > >> >>>>>>> Gary
>> > >> >>>>>>
>> > >> >>>>>>
>> > >> ---------------------------------------------------------------------
>> > >> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > >> >>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>> > >> >>>>>>
>> > >> >>>>>>
>> > >> >>>>>
>> > >> >>>
>> > >> >>>
>> > >> >>>
>> > >> >>> --
>> > >> >>> Matt Sicker <bo...@gmail.com>
>> > >> >>>
>> > >> >>>
>> > ---------------------------------------------------------------------
>> > >> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > >> >>> For additional commands, e-mail: dev-help@commons.apache.org
>> > >> >>>
>> > >> >>
>> > >> >>
>> > >> >>
>> ---------------------------------------------------------------------
>> > >> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > >> >> For additional commands, e-mail: dev-help@commons.apache.org
>> > >> >>
>> > >> >
>> > >> >
>> > >> > --
>> > >> > Matt Sicker <bo...@gmail.com>
>> > >> >
>> > >> >
>> ---------------------------------------------------------------------
>> > >> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > >> > For additional commands, e-mail: dev-help@commons.apache.org
>> > >> >
>> > >>
>> > >>
>> > >> ---------------------------------------------------------------------
>> > >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > >> For additional commands, e-mail: dev-help@commons.apache.org
>> > >>
>> > >>
>> >
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Jul 7, 2020 at 7:14 PM Matt Sicker <bo...@gmail.com> wrote:
> JCIP seems to call this idea a monitor, but that’s also the general
> implicit locking mechanism in Java.
>
Hi Matt,
Right, and it is tempting to rename LockingVisitors.AbstractLockVisitor's
'object' ivar to 'monitor' but that is somewhat misleading because it is
not that object that you are locking, instead you are locking an actual
lock and then visiting that object. So I am inclined to keep that part but
Monitor could be in the class name somewhere.
Gary
> On Tue, Jul 7, 2020 at 17:56 Gary Gregory <ga...@gmail.com> wrote:
>
> > Typo: LockingVisitors.
> >
> > On Tue, Jul 7, 2020 at 6:56 PM Gary Gregory <ga...@gmail.com>
> > wrote:
> >
> > > In the PR https://github.com/apache/commons-lang/pull/559 I am going
> > > with "LockingVistors".
> > >
> > > Gary
> > >
> > > On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com>
> wrote:
> > >
> > >> I’m not all that familiar with that area of academia. Let me see what
> I
> > >> can dig up
> > >>
> > >> -Rob
> > >>
> > >> > On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
> > >> >
> > >> > Are there any academic references for this? Or even something from
> > >> > Java Concurrency in Practice? Those are good sources for names
> around
> > >> > concurrency.
> > >> >
> > >> > On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com>
> wrote:
> > >> >>
> > >> >> Why not make the name “ThreadedLambdaSynchronizer” or something
> like
> > >> that….just something more descriptive here that get’s closer to what
> the
> > >> intended functionality is.
> > >> >>
> > >> >> That said, I’m not particularly tied to the name
> > >> “ThreadedLambdaSynchronizer” just spitballing here for something more
> > >> descriptive than “Lock”
> > >> >>
> > >> >> -Rob
> > >> >>
> > >> >>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com>
> wrote:
> > >> >>>
> > >> >>> Now that starts to sound like Apache Groovy or Kotlin.
> > >> >>>
> > >> >>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com>
> > wrote:
> > >> >>>>
> > >> >>>> soemtimes I really wish to rewrite/add some functions in jdk
> > >> directly...
> > >> >>>> especially for reusing some package private static functions...
> > >> >>>>
> > >> >>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
> > >> >>>>
> > >> >>>>> I'm not sure talking to the JDK folks is helpful IMO. We are
> still
> > >> >>>>> targeting Java 8. The customers I deal with are migrating from 8
> > to
> > >> 11, and
> > >> >>>>> Java 11 is not everywhere our customers are. So talking about
> > >> something
> > >> >>>>> that might end up in Java... 25 seems to be not in our user's
> best
> > >> or
> > >> >>>>> immediate interest. It might be good for Java in the long oh so
> > >> long term
> > >> >>>>> of course.
> > >> >>>>>
> > >> >>>>> Gary
> > >> >>>>>
> > >> >>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <
> chtompki@gmail.com>
> > >> wrote:
> > >> >>>>>
> > >> >>>>>> At first look, I’m a little surprised we’re trying to take on
> > this
> > >> >>>>>> functionality. Has anyone reached out to the JDK guys to see if
> > >> they’d be
> > >> >>>>>> interested in having it in the JDK? That said, if we approach
> it
> > >> from
> > >> >>>>> that
> > >> >>>>>> path, we would lose the functionality in older versions of
> java.
> > So
> > >> >>>>> maybe I
> > >> >>>>>> just talked myself out of the idea of putting it in the JDK....
> > >> >>>>>>
> > >> >>>>>> Just wanted to stream of consciousness my initial gut vibe.
> > >> >>>>>>
> > >> >>>>>> Cheers,
> > >> >>>>>> -Rob
> > >> >>>>>>
> > >> >>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <
> > >> garydgregory@gmail.com>
> > >> >>>>>> wrote:
> > >> >>>>>>>
> > >> >>>>>>> Hi All:
> > >> >>>>>>>
> > >> >>>>>>> I know email is a challenging medium for code reviews, so
> please
> > >> >>>>> consider
> > >> >>>>>>> these comments coming from my best intentions, constructive
> and
> > >> caring
> > >> >>>>>> ;-)
> > >> >>>>>>> Also please excuse the meandering nature of this post.
> > >> >>>>>>>
> > >> >>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock
> > needs
> > >> >>>>> better
> > >> >>>>>>> names IMO, not just the class name. There is already a JRE
> > >> interface
> > >> >>>>>> called
> > >> >>>>>>> Lock so this class name is confusing (to me.)
> > >> >>>>>>>
> > >> >>>>>>> The Javadoc reads in part:
> > >> >>>>>>>
> > >> >>>>>>> * This class implements a wrapper for a locked (hidden)
> > object,
> > >> and
> > >> >>>>>>> * provides the means to access it. The basic idea, is that
> the
> > >> user
> > >> >>>>>>> * code forsakes all references to the locked object, using
> > only
> > >> the
> > >> >>>>>>> * wrapper object, and the accessor methods
> > >> >>>>>>>
> > >> >>>>>>> But this is not the case, the object itself is not locked in
> > >> >>>>>>> the traditional sense with a monitor through a synchronized
> > >> block, so
> > >> >>>>> we
> > >> >>>>>>> need to update the Javadoc as well.
> > >> >>>>>>>
> > >> >>>>>>> To me, this is more about executing blocks of code (through
> > >> lambdas)
> > >> >>>>>> which
> > >> >>>>>>> then get 'safe' (via locking) access to an underlying object.
> > This
> > >> >>>>> tells
> > >> >>>>>> me
> > >> >>>>>>> the class is more about the functional interfaces than about a
> > >> domain
> > >> >>>>>>> "Object", hence the name "Lock" is not the best. It's really
> > more
> > >> of a
> > >> >>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
> > >> >>>>>>> Instead, maybe:
> > >> >>>>>>> - StampledLockVisitor (more specific)
> > >> >>>>>>> - LockingVisitor (more general)
> > >> >>>>>>> - SafeObject (vague)
> > >> >>>>>>> - ObjectLocker (vague)
> > >> >>>>>>> - rejected: LockedObject since the object itself is not
> locked.
> > >> >>>>>>> - ?
> > >> >>>>>>>
> > >> >>>>>>> What is also confusing IMO is that the instance variable for
> the
> > >> object
> > >> >>>>>> is
> > >> >>>>>>> called "lockedObject" but the object is in fact NOT locked all
> > the
> > >> >>>>> time.
> > >> >>>>>> So
> > >> >>>>>>> that needs to be renamed IMO:
> > >> >>>>>>> - object (the simplest)
> > >> >>>>>>> - subject
> > >> >>>>>>> - domain
> > >> >>>>>>> - target
> > >> >>>>>>> - ?
> > >> >>>>>>>
> > >> >>>>>>> In the same vein, the StampedLock is named "lock" which is
> also
> > >> >>>>> confusing
> > >> >>>>>>> since StampedLock does not implement Lock.
> > >> >>>>>>>
> > >> >>>>>>> Why can't the domain object be null, it's never used by the
> > >> framework?
> > >> >>>>>>> Why this:
> > >> >>>>>>> if (t == lockedObject) {
> > >> >>>>>>> throw new IllegalStateException("The returned
> > >> object
> > >> >>>>>>> is, in fact, the hidden object.");
> > >> >>>>>>> }
> > >> >>>>>>> ?
> > >> >>>>>>> This seems like an application specific constraint that has
> > >> nothing to
> > >> >>>>> do
> > >> >>>>>>> with the framework.
> > >> >>>>>>>
> > >> >>>>>>> Now that I've considered the above, the API Locks.lock(O) is
> > >> really
> > >> >>>>>>> misnamed, because it does not lock anything, it's a factory
> > >> method.
> > >> >>>>>>>
> > >> >>>>>>> Stepping back even more, since there is only a static inner
> > class
> > >> in
> > >> >>>>>> Locks,
> > >> >>>>>>> and no-hint that alternative implementations for different
> kind
> > of
> > >> >>>>> locks
> > >> >>>>>>> are possible, I would say we do not need Locks, all we need is
> > >> what is
> > >> >>>>>> now
> > >> >>>>>>> called Lock.
> > >> >>>>>>>
> > >> >>>>>>> It's not clear why some methods are protected, I would make
> > those
> > >> >>>>>> private.
> > >> >>>>>>> It's not like I can use or plugin a Lock,
> > ReentrantReadWriteLock,
> > >> a
> > >> >>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock.
> I
> > >> would
> > >> >>>>>> then
> > >> >>>>>>> make the current Lock class a standalone class which can then
> be
> > >> used
> > >> >>>>>> later
> > >> >>>>>>> from a factory class (now Locks) where we can then fill in
> with
> > >> >>>>> different
> > >> >>>>>>> implementations.
> > >> >>>>>>>
> > >> >>>>>>> The Javadoc talks about a "hidden" object but it is not hidden
> > >> since it
> > >> >>>>>> is
> > >> >>>>>>> passed to all visitors!
> > >> >>>>>>>
> > >> >>>>>>> This test assumption is wrong:
> > >> >>>>>>>
> > >> >>>>>>> If our threads are running concurrently, then we expect
> to
> > >> be
> > >> >>>>>>> faster
> > >> >>>>>>> than running one after the other.
> > >> >>>>>>>
> > >> >>>>>>> The VM or Java spec makes no such guarantee and the tests have
> > no
> > >> >>>>> control
> > >> >>>>>>> over VM scheduling. There are cases where this will fail when
> > >> under
> > >> >>>>> heavy
> > >> >>>>>>> load for example where the cost of additional threads becomes
> > >> >>>>>> overwhelming.
> > >> >>>>>>>
> > >> >>>>>>> Another item I am quite doubtful about is hiding checked
> > >> exceptions by
> > >> >>>>>>> rethrowning them as unchecked. I'd consider this an
> > anti-pattern.
> > >> If I
> > >> >>>>> am
> > >> >>>>>>> using one of our "Failing" interfaces it is because I am
> > >> expecting it
> > >> >>>>> to
> > >> >>>>>>> fail. Perhaps this could be made smoother by refactoring to
> pass
> > >> in a
> > >> >>>>>>> lambda for an exception handler.
> > >> >>>>>>>
> > >> >>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc
> > needs
> > >> >>>>> work):
> > >> >>>>>>> https://github.com/apache/commons-lang/pull/559
> > >> >>>>>>>
> > >> >>>>>>> Not as important as the above:
> > >> >>>>>>> The example in the Javadoc uses logging as its domain
> subject, a
> > >> >>>>>> "logging"
> > >> >>>>>>> API (PrintStream) which is not a good example IMO. Logging
> > >> frameworks
> > >> >>>>>>> today like Log4j handle multi-threaded applications normally
> > >> without
> > >> >>>>>> having
> > >> >>>>>>> developers meddle in it. Yes, I understand it's a simple
> example
> > >> but I
> > >> >>>>> am
> > >> >>>>>>> hoping we can come up with something more realistic or useful.
> > >> >>>>>>>
> > >> >>>>>>> Thank you,
> > >> >>>>>>> Gary
> > >> >>>>>>
> > >> >>>>>>
> > >> ---------------------------------------------------------------------
> > >> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> >>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> > >> >>>>>>
> > >> >>>>>>
> > >> >>>>>
> > >> >>>
> > >> >>>
> > >> >>>
> > >> >>> --
> > >> >>> Matt Sicker <bo...@gmail.com>
> > >> >>>
> > >> >>>
> > ---------------------------------------------------------------------
> > >> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> >>> For additional commands, e-mail: dev-help@commons.apache.org
> > >> >>>
> > >> >>
> > >> >>
> > >> >>
> ---------------------------------------------------------------------
> > >> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> >> For additional commands, e-mail: dev-help@commons.apache.org
> > >> >>
> > >> >
> > >> >
> > >> > --
> > >> > Matt Sicker <bo...@gmail.com>
> > >> >
> > >> >
> ---------------------------------------------------------------------
> > >> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> > For additional commands, e-mail: dev-help@commons.apache.org
> > >> >
> > >>
> > >>
> > >> ---------------------------------------------------------------------
> > >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> For additional commands, e-mail: dev-help@commons.apache.org
> > >>
> > >>
> >
> --
> Matt Sicker <bo...@gmail.com>
>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Matt Sicker <bo...@gmail.com>.
JCIP seems to call this idea a monitor, but that’s also the general
implicit locking mechanism in Java.
On Tue, Jul 7, 2020 at 17:56 Gary Gregory <ga...@gmail.com> wrote:
> Typo: LockingVisitors.
>
> On Tue, Jul 7, 2020 at 6:56 PM Gary Gregory <ga...@gmail.com>
> wrote:
>
> > In the PR https://github.com/apache/commons-lang/pull/559 I am going
> > with "LockingVistors".
> >
> > Gary
> >
> > On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com> wrote:
> >
> >> I’m not all that familiar with that area of academia. Let me see what I
> >> can dig up
> >>
> >> -Rob
> >>
> >> > On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
> >> >
> >> > Are there any academic references for this? Or even something from
> >> > Java Concurrency in Practice? Those are good sources for names around
> >> > concurrency.
> >> >
> >> > On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
> >> >>
> >> >> Why not make the name “ThreadedLambdaSynchronizer” or something like
> >> that….just something more descriptive here that get’s closer to what the
> >> intended functionality is.
> >> >>
> >> >> That said, I’m not particularly tied to the name
> >> “ThreadedLambdaSynchronizer” just spitballing here for something more
> >> descriptive than “Lock”
> >> >>
> >> >> -Rob
> >> >>
> >> >>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
> >> >>>
> >> >>> Now that starts to sound like Apache Groovy or Kotlin.
> >> >>>
> >> >>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com>
> wrote:
> >> >>>>
> >> >>>> soemtimes I really wish to rewrite/add some functions in jdk
> >> directly...
> >> >>>> especially for reusing some package private static functions...
> >> >>>>
> >> >>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
> >> >>>>
> >> >>>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
> >> >>>>> targeting Java 8. The customers I deal with are migrating from 8
> to
> >> 11, and
> >> >>>>> Java 11 is not everywhere our customers are. So talking about
> >> something
> >> >>>>> that might end up in Java... 25 seems to be not in our user's best
> >> or
> >> >>>>> immediate interest. It might be good for Java in the long oh so
> >> long term
> >> >>>>> of course.
> >> >>>>>
> >> >>>>> Gary
> >> >>>>>
> >> >>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com>
> >> wrote:
> >> >>>>>
> >> >>>>>> At first look, I’m a little surprised we’re trying to take on
> this
> >> >>>>>> functionality. Has anyone reached out to the JDK guys to see if
> >> they’d be
> >> >>>>>> interested in having it in the JDK? That said, if we approach it
> >> from
> >> >>>>> that
> >> >>>>>> path, we would lose the functionality in older versions of java.
> So
> >> >>>>> maybe I
> >> >>>>>> just talked myself out of the idea of putting it in the JDK....
> >> >>>>>>
> >> >>>>>> Just wanted to stream of consciousness my initial gut vibe.
> >> >>>>>>
> >> >>>>>> Cheers,
> >> >>>>>> -Rob
> >> >>>>>>
> >> >>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <
> >> garydgregory@gmail.com>
> >> >>>>>> wrote:
> >> >>>>>>>
> >> >>>>>>> Hi All:
> >> >>>>>>>
> >> >>>>>>> I know email is a challenging medium for code reviews, so please
> >> >>>>> consider
> >> >>>>>>> these comments coming from my best intentions, constructive and
> >> caring
> >> >>>>>> ;-)
> >> >>>>>>> Also please excuse the meandering nature of this post.
> >> >>>>>>>
> >> >>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock
> needs
> >> >>>>> better
> >> >>>>>>> names IMO, not just the class name. There is already a JRE
> >> interface
> >> >>>>>> called
> >> >>>>>>> Lock so this class name is confusing (to me.)
> >> >>>>>>>
> >> >>>>>>> The Javadoc reads in part:
> >> >>>>>>>
> >> >>>>>>> * This class implements a wrapper for a locked (hidden)
> object,
> >> and
> >> >>>>>>> * provides the means to access it. The basic idea, is that the
> >> user
> >> >>>>>>> * code forsakes all references to the locked object, using
> only
> >> the
> >> >>>>>>> * wrapper object, and the accessor methods
> >> >>>>>>>
> >> >>>>>>> But this is not the case, the object itself is not locked in
> >> >>>>>>> the traditional sense with a monitor through a synchronized
> >> block, so
> >> >>>>> we
> >> >>>>>>> need to update the Javadoc as well.
> >> >>>>>>>
> >> >>>>>>> To me, this is more about executing blocks of code (through
> >> lambdas)
> >> >>>>>> which
> >> >>>>>>> then get 'safe' (via locking) access to an underlying object.
> This
> >> >>>>> tells
> >> >>>>>> me
> >> >>>>>>> the class is more about the functional interfaces than about a
> >> domain
> >> >>>>>>> "Object", hence the name "Lock" is not the best. It's really
> more
> >> of a
> >> >>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
> >> >>>>>>> Instead, maybe:
> >> >>>>>>> - StampledLockVisitor (more specific)
> >> >>>>>>> - LockingVisitor (more general)
> >> >>>>>>> - SafeObject (vague)
> >> >>>>>>> - ObjectLocker (vague)
> >> >>>>>>> - rejected: LockedObject since the object itself is not locked.
> >> >>>>>>> - ?
> >> >>>>>>>
> >> >>>>>>> What is also confusing IMO is that the instance variable for the
> >> object
> >> >>>>>> is
> >> >>>>>>> called "lockedObject" but the object is in fact NOT locked all
> the
> >> >>>>> time.
> >> >>>>>> So
> >> >>>>>>> that needs to be renamed IMO:
> >> >>>>>>> - object (the simplest)
> >> >>>>>>> - subject
> >> >>>>>>> - domain
> >> >>>>>>> - target
> >> >>>>>>> - ?
> >> >>>>>>>
> >> >>>>>>> In the same vein, the StampedLock is named "lock" which is also
> >> >>>>> confusing
> >> >>>>>>> since StampedLock does not implement Lock.
> >> >>>>>>>
> >> >>>>>>> Why can't the domain object be null, it's never used by the
> >> framework?
> >> >>>>>>> Why this:
> >> >>>>>>> if (t == lockedObject) {
> >> >>>>>>> throw new IllegalStateException("The returned
> >> object
> >> >>>>>>> is, in fact, the hidden object.");
> >> >>>>>>> }
> >> >>>>>>> ?
> >> >>>>>>> This seems like an application specific constraint that has
> >> nothing to
> >> >>>>> do
> >> >>>>>>> with the framework.
> >> >>>>>>>
> >> >>>>>>> Now that I've considered the above, the API Locks.lock(O) is
> >> really
> >> >>>>>>> misnamed, because it does not lock anything, it's a factory
> >> method.
> >> >>>>>>>
> >> >>>>>>> Stepping back even more, since there is only a static inner
> class
> >> in
> >> >>>>>> Locks,
> >> >>>>>>> and no-hint that alternative implementations for different kind
> of
> >> >>>>> locks
> >> >>>>>>> are possible, I would say we do not need Locks, all we need is
> >> what is
> >> >>>>>> now
> >> >>>>>>> called Lock.
> >> >>>>>>>
> >> >>>>>>> It's not clear why some methods are protected, I would make
> those
> >> >>>>>> private.
> >> >>>>>>> It's not like I can use or plugin a Lock,
> ReentrantReadWriteLock,
> >> a
> >> >>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I
> >> would
> >> >>>>>> then
> >> >>>>>>> make the current Lock class a standalone class which can then be
> >> used
> >> >>>>>> later
> >> >>>>>>> from a factory class (now Locks) where we can then fill in with
> >> >>>>> different
> >> >>>>>>> implementations.
> >> >>>>>>>
> >> >>>>>>> The Javadoc talks about a "hidden" object but it is not hidden
> >> since it
> >> >>>>>> is
> >> >>>>>>> passed to all visitors!
> >> >>>>>>>
> >> >>>>>>> This test assumption is wrong:
> >> >>>>>>>
> >> >>>>>>> If our threads are running concurrently, then we expect to
> >> be
> >> >>>>>>> faster
> >> >>>>>>> than running one after the other.
> >> >>>>>>>
> >> >>>>>>> The VM or Java spec makes no such guarantee and the tests have
> no
> >> >>>>> control
> >> >>>>>>> over VM scheduling. There are cases where this will fail when
> >> under
> >> >>>>> heavy
> >> >>>>>>> load for example where the cost of additional threads becomes
> >> >>>>>> overwhelming.
> >> >>>>>>>
> >> >>>>>>> Another item I am quite doubtful about is hiding checked
> >> exceptions by
> >> >>>>>>> rethrowning them as unchecked. I'd consider this an
> anti-pattern.
> >> If I
> >> >>>>> am
> >> >>>>>>> using one of our "Failing" interfaces it is because I am
> >> expecting it
> >> >>>>> to
> >> >>>>>>> fail. Perhaps this could be made smoother by refactoring to pass
> >> in a
> >> >>>>>>> lambda for an exception handler.
> >> >>>>>>>
> >> >>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc
> needs
> >> >>>>> work):
> >> >>>>>>> https://github.com/apache/commons-lang/pull/559
> >> >>>>>>>
> >> >>>>>>> Not as important as the above:
> >> >>>>>>> The example in the Javadoc uses logging as its domain subject, a
> >> >>>>>> "logging"
> >> >>>>>>> API (PrintStream) which is not a good example IMO. Logging
> >> frameworks
> >> >>>>>>> today like Log4j handle multi-threaded applications normally
> >> without
> >> >>>>>> having
> >> >>>>>>> developers meddle in it. Yes, I understand it's a simple example
> >> but I
> >> >>>>> am
> >> >>>>>>> hoping we can come up with something more realistic or useful.
> >> >>>>>>>
> >> >>>>>>> Thank you,
> >> >>>>>>> Gary
> >> >>>>>>
> >> >>>>>>
> >> ---------------------------------------------------------------------
> >> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>>>>>
> >> >>>>>>
> >> >>>>>
> >> >>>
> >> >>>
> >> >>>
> >> >>> --
> >> >>> Matt Sicker <bo...@gmail.com>
> >> >>>
> >> >>>
> ---------------------------------------------------------------------
> >> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>>
> >> >>
> >> >>
> >> >> ---------------------------------------------------------------------
> >> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>
> >> >
> >> >
> >> > --
> >> > Matt Sicker <bo...@gmail.com>
> >> >
> >> > ---------------------------------------------------------------------
> >> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> > For additional commands, e-mail: dev-help@commons.apache.org
> >> >
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >>
>
--
Matt Sicker <bo...@gmail.com>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
Typo: LockingVisitors.
On Tue, Jul 7, 2020 at 6:56 PM Gary Gregory <ga...@gmail.com> wrote:
> In the PR https://github.com/apache/commons-lang/pull/559 I am going
> with "LockingVistors".
>
> Gary
>
> On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com> wrote:
>
>> I’m not all that familiar with that area of academia. Let me see what I
>> can dig up
>>
>> -Rob
>>
>> > On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
>> >
>> > Are there any academic references for this? Or even something from
>> > Java Concurrency in Practice? Those are good sources for names around
>> > concurrency.
>> >
>> > On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
>> >>
>> >> Why not make the name “ThreadedLambdaSynchronizer” or something like
>> that….just something more descriptive here that get’s closer to what the
>> intended functionality is.
>> >>
>> >> That said, I’m not particularly tied to the name
>> “ThreadedLambdaSynchronizer” just spitballing here for something more
>> descriptive than “Lock”
>> >>
>> >> -Rob
>> >>
>> >>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
>> >>>
>> >>> Now that starts to sound like Apache Groovy or Kotlin.
>> >>>
>> >>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com> wrote:
>> >>>>
>> >>>> soemtimes I really wish to rewrite/add some functions in jdk
>> directly...
>> >>>> especially for reusing some package private static functions...
>> >>>>
>> >>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
>> >>>>
>> >>>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
>> >>>>> targeting Java 8. The customers I deal with are migrating from 8 to
>> 11, and
>> >>>>> Java 11 is not everywhere our customers are. So talking about
>> something
>> >>>>> that might end up in Java... 25 seems to be not in our user's best
>> or
>> >>>>> immediate interest. It might be good for Java in the long oh so
>> long term
>> >>>>> of course.
>> >>>>>
>> >>>>> Gary
>> >>>>>
>> >>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com>
>> wrote:
>> >>>>>
>> >>>>>> At first look, I’m a little surprised we’re trying to take on this
>> >>>>>> functionality. Has anyone reached out to the JDK guys to see if
>> they’d be
>> >>>>>> interested in having it in the JDK? That said, if we approach it
>> from
>> >>>>> that
>> >>>>>> path, we would lose the functionality in older versions of java. So
>> >>>>> maybe I
>> >>>>>> just talked myself out of the idea of putting it in the JDK....
>> >>>>>>
>> >>>>>> Just wanted to stream of consciousness my initial gut vibe.
>> >>>>>>
>> >>>>>> Cheers,
>> >>>>>> -Rob
>> >>>>>>
>> >>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <
>> garydgregory@gmail.com>
>> >>>>>> wrote:
>> >>>>>>>
>> >>>>>>> Hi All:
>> >>>>>>>
>> >>>>>>> I know email is a challenging medium for code reviews, so please
>> >>>>> consider
>> >>>>>>> these comments coming from my best intentions, constructive and
>> caring
>> >>>>>> ;-)
>> >>>>>>> Also please excuse the meandering nature of this post.
>> >>>>>>>
>> >>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
>> >>>>> better
>> >>>>>>> names IMO, not just the class name. There is already a JRE
>> interface
>> >>>>>> called
>> >>>>>>> Lock so this class name is confusing (to me.)
>> >>>>>>>
>> >>>>>>> The Javadoc reads in part:
>> >>>>>>>
>> >>>>>>> * This class implements a wrapper for a locked (hidden) object,
>> and
>> >>>>>>> * provides the means to access it. The basic idea, is that the
>> user
>> >>>>>>> * code forsakes all references to the locked object, using only
>> the
>> >>>>>>> * wrapper object, and the accessor methods
>> >>>>>>>
>> >>>>>>> But this is not the case, the object itself is not locked in
>> >>>>>>> the traditional sense with a monitor through a synchronized
>> block, so
>> >>>>> we
>> >>>>>>> need to update the Javadoc as well.
>> >>>>>>>
>> >>>>>>> To me, this is more about executing blocks of code (through
>> lambdas)
>> >>>>>> which
>> >>>>>>> then get 'safe' (via locking) access to an underlying object. This
>> >>>>> tells
>> >>>>>> me
>> >>>>>>> the class is more about the functional interfaces than about a
>> domain
>> >>>>>>> "Object", hence the name "Lock" is not the best. It's really more
>> of a
>> >>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
>> >>>>>>> Instead, maybe:
>> >>>>>>> - StampledLockVisitor (more specific)
>> >>>>>>> - LockingVisitor (more general)
>> >>>>>>> - SafeObject (vague)
>> >>>>>>> - ObjectLocker (vague)
>> >>>>>>> - rejected: LockedObject since the object itself is not locked.
>> >>>>>>> - ?
>> >>>>>>>
>> >>>>>>> What is also confusing IMO is that the instance variable for the
>> object
>> >>>>>> is
>> >>>>>>> called "lockedObject" but the object is in fact NOT locked all the
>> >>>>> time.
>> >>>>>> So
>> >>>>>>> that needs to be renamed IMO:
>> >>>>>>> - object (the simplest)
>> >>>>>>> - subject
>> >>>>>>> - domain
>> >>>>>>> - target
>> >>>>>>> - ?
>> >>>>>>>
>> >>>>>>> In the same vein, the StampedLock is named "lock" which is also
>> >>>>> confusing
>> >>>>>>> since StampedLock does not implement Lock.
>> >>>>>>>
>> >>>>>>> Why can't the domain object be null, it's never used by the
>> framework?
>> >>>>>>> Why this:
>> >>>>>>> if (t == lockedObject) {
>> >>>>>>> throw new IllegalStateException("The returned
>> object
>> >>>>>>> is, in fact, the hidden object.");
>> >>>>>>> }
>> >>>>>>> ?
>> >>>>>>> This seems like an application specific constraint that has
>> nothing to
>> >>>>> do
>> >>>>>>> with the framework.
>> >>>>>>>
>> >>>>>>> Now that I've considered the above, the API Locks.lock(O) is
>> really
>> >>>>>>> misnamed, because it does not lock anything, it's a factory
>> method.
>> >>>>>>>
>> >>>>>>> Stepping back even more, since there is only a static inner class
>> in
>> >>>>>> Locks,
>> >>>>>>> and no-hint that alternative implementations for different kind of
>> >>>>> locks
>> >>>>>>> are possible, I would say we do not need Locks, all we need is
>> what is
>> >>>>>> now
>> >>>>>>> called Lock.
>> >>>>>>>
>> >>>>>>> It's not clear why some methods are protected, I would make those
>> >>>>>> private.
>> >>>>>>> It's not like I can use or plugin a Lock, ReentrantReadWriteLock,
>> a
>> >>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I
>> would
>> >>>>>> then
>> >>>>>>> make the current Lock class a standalone class which can then be
>> used
>> >>>>>> later
>> >>>>>>> from a factory class (now Locks) where we can then fill in with
>> >>>>> different
>> >>>>>>> implementations.
>> >>>>>>>
>> >>>>>>> The Javadoc talks about a "hidden" object but it is not hidden
>> since it
>> >>>>>> is
>> >>>>>>> passed to all visitors!
>> >>>>>>>
>> >>>>>>> This test assumption is wrong:
>> >>>>>>>
>> >>>>>>> If our threads are running concurrently, then we expect to
>> be
>> >>>>>>> faster
>> >>>>>>> than running one after the other.
>> >>>>>>>
>> >>>>>>> The VM or Java spec makes no such guarantee and the tests have no
>> >>>>> control
>> >>>>>>> over VM scheduling. There are cases where this will fail when
>> under
>> >>>>> heavy
>> >>>>>>> load for example where the cost of additional threads becomes
>> >>>>>> overwhelming.
>> >>>>>>>
>> >>>>>>> Another item I am quite doubtful about is hiding checked
>> exceptions by
>> >>>>>>> rethrowning them as unchecked. I'd consider this an anti-pattern.
>> If I
>> >>>>> am
>> >>>>>>> using one of our "Failing" interfaces it is because I am
>> expecting it
>> >>>>> to
>> >>>>>>> fail. Perhaps this could be made smoother by refactoring to pass
>> in a
>> >>>>>>> lambda for an exception handler.
>> >>>>>>>
>> >>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc needs
>> >>>>> work):
>> >>>>>>> https://github.com/apache/commons-lang/pull/559
>> >>>>>>>
>> >>>>>>> Not as important as the above:
>> >>>>>>> The example in the Javadoc uses logging as its domain subject, a
>> >>>>>> "logging"
>> >>>>>>> API (PrintStream) which is not a good example IMO. Logging
>> frameworks
>> >>>>>>> today like Log4j handle multi-threaded applications normally
>> without
>> >>>>>> having
>> >>>>>>> developers meddle in it. Yes, I understand it's a simple example
>> but I
>> >>>>> am
>> >>>>>>> hoping we can come up with something more realistic or useful.
>> >>>>>>>
>> >>>>>>> Thank you,
>> >>>>>>> Gary
>> >>>>>>
>> >>>>>>
>> ---------------------------------------------------------------------
>> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>>>>
>> >>>>>>
>> >>>>>
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Matt Sicker <bo...@gmail.com>
>> >>>
>> >>> ---------------------------------------------------------------------
>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>
>> >>
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >> For additional commands, e-mail: dev-help@commons.apache.org
>> >>
>> >
>> >
>> > --
>> > Matt Sicker <bo...@gmail.com>
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Jul 8, 2020 at 10:13 AM Gary Gregory <ga...@gmail.com> wrote:
>
>
> On Wed, Jul 8, 2020, 09:32 Rob Tompkins <ch...@gmail.com> wrote:
>
>>
>>
>> > On Jul 7, 2020, at 6:56 PM, Gary Gregory <ga...@gmail.com>
>> wrote:
>> >
>> > In the PR https://github.com/apache/commons-lang/pull/559 I am going
>> with
>> > "LockingVistors".
>>
>> I like this name because of its brevity yet clarity.
>>
>
> Let's go for that then. I'll merge the PR later today after I give it
> another review.
>
Merged, please review, discuss, or whatnot. I'd like to do an RC very soon,
semi-depending on the resolution of
https://github.com/apache/commons-parent/pull/9
> Gary
>
>>
>> -Rob
>>
>> >
>> > Gary
>> >
>> > On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com> wrote:
>> >
>> >> I’m not all that familiar with that area of academia. Let me see what I
>> >> can dig up
>> >>
>> >> -Rob
>> >>
>> >>> On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
>> >>>
>> >>> Are there any academic references for this? Or even something from
>> >>> Java Concurrency in Practice? Those are good sources for names around
>> >>> concurrency.
>> >>>
>> >>> On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
>> >>>>
>> >>>> Why not make the name “ThreadedLambdaSynchronizer” or something like
>> >> that….just something more descriptive here that get’s closer to what
>> the
>> >> intended functionality is.
>> >>>>
>> >>>> That said, I’m not particularly tied to the name
>> >> “ThreadedLambdaSynchronizer” just spitballing here for something more
>> >> descriptive than “Lock”
>> >>>>
>> >>>> -Rob
>> >>>>
>> >>>>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
>> >>>>>
>> >>>>> Now that starts to sound like Apache Groovy or Kotlin.
>> >>>>>
>> >>>>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com>
>> wrote:
>> >>>>>>
>> >>>>>> soemtimes I really wish to rewrite/add some functions in jdk
>> >> directly...
>> >>>>>> especially for reusing some package private static functions...
>> >>>>>>
>> >>>>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
>> >>>>>>
>> >>>>>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
>> >>>>>>> targeting Java 8. The customers I deal with are migrating from 8
>> to
>> >> 11, and
>> >>>>>>> Java 11 is not everywhere our customers are. So talking about
>> >> something
>> >>>>>>> that might end up in Java... 25 seems to be not in our user's
>> best or
>> >>>>>>> immediate interest. It might be good for Java in the long oh so
>> long
>> >> term
>> >>>>>>> of course.
>> >>>>>>>
>> >>>>>>> Gary
>> >>>>>>>
>> >>>>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com>
>> >> wrote:
>> >>>>>>>
>> >>>>>>>> At first look, I’m a little surprised we’re trying to take on
>> this
>> >>>>>>>> functionality. Has anyone reached out to the JDK guys to see if
>> >> they’d be
>> >>>>>>>> interested in having it in the JDK? That said, if we approach it
>> >> from
>> >>>>>>> that
>> >>>>>>>> path, we would lose the functionality in older versions of java.
>> So
>> >>>>>>> maybe I
>> >>>>>>>> just talked myself out of the idea of putting it in the JDK....
>> >>>>>>>>
>> >>>>>>>> Just wanted to stream of consciousness my initial gut vibe.
>> >>>>>>>>
>> >>>>>>>> Cheers,
>> >>>>>>>> -Rob
>> >>>>>>>>
>> >>>>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <
>> garydgregory@gmail.com
>> >>>
>> >>>>>>>> wrote:
>> >>>>>>>>>
>> >>>>>>>>> Hi All:
>> >>>>>>>>>
>> >>>>>>>>> I know email is a challenging medium for code reviews, so please
>> >>>>>>> consider
>> >>>>>>>>> these comments coming from my best intentions, constructive and
>> >> caring
>> >>>>>>>> ;-)
>> >>>>>>>>> Also please excuse the meandering nature of this post.
>> >>>>>>>>>
>> >>>>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock
>> needs
>> >>>>>>> better
>> >>>>>>>>> names IMO, not just the class name. There is already a JRE
>> >> interface
>> >>>>>>>> called
>> >>>>>>>>> Lock so this class name is confusing (to me.)
>> >>>>>>>>>
>> >>>>>>>>> The Javadoc reads in part:
>> >>>>>>>>>
>> >>>>>>>>> * This class implements a wrapper for a locked (hidden) object,
>> >> and
>> >>>>>>>>> * provides the means to access it. The basic idea, is that the
>> >> user
>> >>>>>>>>> * code forsakes all references to the locked object, using only
>> >> the
>> >>>>>>>>> * wrapper object, and the accessor methods
>> >>>>>>>>>
>> >>>>>>>>> But this is not the case, the object itself is not locked in
>> >>>>>>>>> the traditional sense with a monitor through a synchronized
>> block,
>> >> so
>> >>>>>>> we
>> >>>>>>>>> need to update the Javadoc as well.
>> >>>>>>>>>
>> >>>>>>>>> To me, this is more about executing blocks of code (through
>> >> lambdas)
>> >>>>>>>> which
>> >>>>>>>>> then get 'safe' (via locking) access to an underlying object.
>> This
>> >>>>>>> tells
>> >>>>>>>> me
>> >>>>>>>>> the class is more about the functional interfaces than about a
>> >> domain
>> >>>>>>>>> "Object", hence the name "Lock" is not the best. It's really
>> more
>> >> of a
>> >>>>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
>> >>>>>>>>> Instead, maybe:
>> >>>>>>>>> - StampledLockVisitor (more specific)
>> >>>>>>>>> - LockingVisitor (more general)
>> >>>>>>>>> - SafeObject (vague)
>> >>>>>>>>> - ObjectLocker (vague)
>> >>>>>>>>> - rejected: LockedObject since the object itself is not locked.
>> >>>>>>>>> - ?
>> >>>>>>>>>
>> >>>>>>>>> What is also confusing IMO is that the instance variable for the
>> >> object
>> >>>>>>>> is
>> >>>>>>>>> called "lockedObject" but the object is in fact NOT locked all
>> the
>> >>>>>>> time.
>> >>>>>>>> So
>> >>>>>>>>> that needs to be renamed IMO:
>> >>>>>>>>> - object (the simplest)
>> >>>>>>>>> - subject
>> >>>>>>>>> - domain
>> >>>>>>>>> - target
>> >>>>>>>>> - ?
>> >>>>>>>>>
>> >>>>>>>>> In the same vein, the StampedLock is named "lock" which is also
>> >>>>>>> confusing
>> >>>>>>>>> since StampedLock does not implement Lock.
>> >>>>>>>>>
>> >>>>>>>>> Why can't the domain object be null, it's never used by the
>> >> framework?
>> >>>>>>>>> Why this:
>> >>>>>>>>> if (t == lockedObject) {
>> >>>>>>>>> throw new IllegalStateException("The returned
>> >> object
>> >>>>>>>>> is, in fact, the hidden object.");
>> >>>>>>>>> }
>> >>>>>>>>> ?
>> >>>>>>>>> This seems like an application specific constraint that has
>> >> nothing to
>> >>>>>>> do
>> >>>>>>>>> with the framework.
>> >>>>>>>>>
>> >>>>>>>>> Now that I've considered the above, the API Locks.lock(O) is
>> really
>> >>>>>>>>> misnamed, because it does not lock anything, it's a factory
>> method.
>> >>>>>>>>>
>> >>>>>>>>> Stepping back even more, since there is only a static inner
>> class
>> >> in
>> >>>>>>>> Locks,
>> >>>>>>>>> and no-hint that alternative implementations for different kind
>> of
>> >>>>>>> locks
>> >>>>>>>>> are possible, I would say we do not need Locks, all we need is
>> >> what is
>> >>>>>>>> now
>> >>>>>>>>> called Lock.
>> >>>>>>>>>
>> >>>>>>>>> It's not clear why some methods are protected, I would make
>> those
>> >>>>>>>> private.
>> >>>>>>>>> It's not like I can use or plugin a Lock,
>> ReentrantReadWriteLock, a
>> >>>>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I
>> >> would
>> >>>>>>>> then
>> >>>>>>>>> make the current Lock class a standalone class which can then be
>> >> used
>> >>>>>>>> later
>> >>>>>>>>> from a factory class (now Locks) where we can then fill in with
>> >>>>>>> different
>> >>>>>>>>> implementations.
>> >>>>>>>>>
>> >>>>>>>>> The Javadoc talks about a "hidden" object but it is not hidden
>> >> since it
>> >>>>>>>> is
>> >>>>>>>>> passed to all visitors!
>> >>>>>>>>>
>> >>>>>>>>> This test assumption is wrong:
>> >>>>>>>>>
>> >>>>>>>>> If our threads are running concurrently, then we expect to
>> be
>> >>>>>>>>> faster
>> >>>>>>>>> than running one after the other.
>> >>>>>>>>>
>> >>>>>>>>> The VM or Java spec makes no such guarantee and the tests have
>> no
>> >>>>>>> control
>> >>>>>>>>> over VM scheduling. There are cases where this will fail when
>> under
>> >>>>>>> heavy
>> >>>>>>>>> load for example where the cost of additional threads becomes
>> >>>>>>>> overwhelming.
>> >>>>>>>>>
>> >>>>>>>>> Another item I am quite doubtful about is hiding checked
>> >> exceptions by
>> >>>>>>>>> rethrowning them as unchecked. I'd consider this an
>> anti-pattern.
>> >> If I
>> >>>>>>> am
>> >>>>>>>>> using one of our "Failing" interfaces it is because I am
>> expecting
>> >> it
>> >>>>>>> to
>> >>>>>>>>> fail. Perhaps this could be made smoother by refactoring to pass
>> >> in a
>> >>>>>>>>> lambda for an exception handler.
>> >>>>>>>>>
>> >>>>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc
>> needs
>> >>>>>>> work):
>> >>>>>>>>> https://github.com/apache/commons-lang/pull/559
>> >>>>>>>>>
>> >>>>>>>>> Not as important as the above:
>> >>>>>>>>> The example in the Javadoc uses logging as its domain subject, a
>> >>>>>>>> "logging"
>> >>>>>>>>> API (PrintStream) which is not a good example IMO. Logging
>> >> frameworks
>> >>>>>>>>> today like Log4j handle multi-threaded applications normally
>> >> without
>> >>>>>>>> having
>> >>>>>>>>> developers meddle in it. Yes, I understand it's a simple example
>> >> but I
>> >>>>>>> am
>> >>>>>>>>> hoping we can come up with something more realistic or useful.
>> >>>>>>>>>
>> >>>>>>>>> Thank you,
>> >>>>>>>>> Gary
>> >>>>>>>>
>> >>>>>>>>
>> >> ---------------------------------------------------------------------
>> >>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> --
>> >>>>> Matt Sicker <bo...@gmail.com>
>> >>>>>
>> >>>>>
>> ---------------------------------------------------------------------
>> >>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>>>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>>>
>> >>>>
>> >>>>
>> >>>> ---------------------------------------------------------------------
>> >>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>>
>> >>>
>> >>>
>> >>> --
>> >>> Matt Sicker <bo...@gmail.com>
>> >>>
>> >>> ---------------------------------------------------------------------
>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>
>> >>
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >> For additional commands, e-mail: dev-help@commons.apache.org
>> >>
>> >>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Jul 8, 2020, 09:32 Rob Tompkins <ch...@gmail.com> wrote:
>
>
> > On Jul 7, 2020, at 6:56 PM, Gary Gregory <ga...@gmail.com> wrote:
> >
> > In the PR https://github.com/apache/commons-lang/pull/559 I am going
> with
> > "LockingVistors".
>
> I like this name because of its brevity yet clarity.
>
Let's go for that then. I'll merge the PR later today after I give it
another review.
Gary
>
> -Rob
>
> >
> > Gary
> >
> > On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com> wrote:
> >
> >> I’m not all that familiar with that area of academia. Let me see what I
> >> can dig up
> >>
> >> -Rob
> >>
> >>> On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
> >>>
> >>> Are there any academic references for this? Or even something from
> >>> Java Concurrency in Practice? Those are good sources for names around
> >>> concurrency.
> >>>
> >>> On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
> >>>>
> >>>> Why not make the name “ThreadedLambdaSynchronizer” or something like
> >> that….just something more descriptive here that get’s closer to what the
> >> intended functionality is.
> >>>>
> >>>> That said, I’m not particularly tied to the name
> >> “ThreadedLambdaSynchronizer” just spitballing here for something more
> >> descriptive than “Lock”
> >>>>
> >>>> -Rob
> >>>>
> >>>>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
> >>>>>
> >>>>> Now that starts to sound like Apache Groovy or Kotlin.
> >>>>>
> >>>>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com>
> wrote:
> >>>>>>
> >>>>>> soemtimes I really wish to rewrite/add some functions in jdk
> >> directly...
> >>>>>> especially for reusing some package private static functions...
> >>>>>>
> >>>>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
> >>>>>>
> >>>>>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
> >>>>>>> targeting Java 8. The customers I deal with are migrating from 8 to
> >> 11, and
> >>>>>>> Java 11 is not everywhere our customers are. So talking about
> >> something
> >>>>>>> that might end up in Java... 25 seems to be not in our user's best
> or
> >>>>>>> immediate interest. It might be good for Java in the long oh so
> long
> >> term
> >>>>>>> of course.
> >>>>>>>
> >>>>>>> Gary
> >>>>>>>
> >>>>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com>
> >> wrote:
> >>>>>>>
> >>>>>>>> At first look, I’m a little surprised we’re trying to take on this
> >>>>>>>> functionality. Has anyone reached out to the JDK guys to see if
> >> they’d be
> >>>>>>>> interested in having it in the JDK? That said, if we approach it
> >> from
> >>>>>>> that
> >>>>>>>> path, we would lose the functionality in older versions of java.
> So
> >>>>>>> maybe I
> >>>>>>>> just talked myself out of the idea of putting it in the JDK....
> >>>>>>>>
> >>>>>>>> Just wanted to stream of consciousness my initial gut vibe.
> >>>>>>>>
> >>>>>>>> Cheers,
> >>>>>>>> -Rob
> >>>>>>>>
> >>>>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <
> garydgregory@gmail.com
> >>>
> >>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi All:
> >>>>>>>>>
> >>>>>>>>> I know email is a challenging medium for code reviews, so please
> >>>>>>> consider
> >>>>>>>>> these comments coming from my best intentions, constructive and
> >> caring
> >>>>>>>> ;-)
> >>>>>>>>> Also please excuse the meandering nature of this post.
> >>>>>>>>>
> >>>>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock
> needs
> >>>>>>> better
> >>>>>>>>> names IMO, not just the class name. There is already a JRE
> >> interface
> >>>>>>>> called
> >>>>>>>>> Lock so this class name is confusing (to me.)
> >>>>>>>>>
> >>>>>>>>> The Javadoc reads in part:
> >>>>>>>>>
> >>>>>>>>> * This class implements a wrapper for a locked (hidden) object,
> >> and
> >>>>>>>>> * provides the means to access it. The basic idea, is that the
> >> user
> >>>>>>>>> * code forsakes all references to the locked object, using only
> >> the
> >>>>>>>>> * wrapper object, and the accessor methods
> >>>>>>>>>
> >>>>>>>>> But this is not the case, the object itself is not locked in
> >>>>>>>>> the traditional sense with a monitor through a synchronized
> block,
> >> so
> >>>>>>> we
> >>>>>>>>> need to update the Javadoc as well.
> >>>>>>>>>
> >>>>>>>>> To me, this is more about executing blocks of code (through
> >> lambdas)
> >>>>>>>> which
> >>>>>>>>> then get 'safe' (via locking) access to an underlying object.
> This
> >>>>>>> tells
> >>>>>>>> me
> >>>>>>>>> the class is more about the functional interfaces than about a
> >> domain
> >>>>>>>>> "Object", hence the name "Lock" is not the best. It's really more
> >> of a
> >>>>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
> >>>>>>>>> Instead, maybe:
> >>>>>>>>> - StampledLockVisitor (more specific)
> >>>>>>>>> - LockingVisitor (more general)
> >>>>>>>>> - SafeObject (vague)
> >>>>>>>>> - ObjectLocker (vague)
> >>>>>>>>> - rejected: LockedObject since the object itself is not locked.
> >>>>>>>>> - ?
> >>>>>>>>>
> >>>>>>>>> What is also confusing IMO is that the instance variable for the
> >> object
> >>>>>>>> is
> >>>>>>>>> called "lockedObject" but the object is in fact NOT locked all
> the
> >>>>>>> time.
> >>>>>>>> So
> >>>>>>>>> that needs to be renamed IMO:
> >>>>>>>>> - object (the simplest)
> >>>>>>>>> - subject
> >>>>>>>>> - domain
> >>>>>>>>> - target
> >>>>>>>>> - ?
> >>>>>>>>>
> >>>>>>>>> In the same vein, the StampedLock is named "lock" which is also
> >>>>>>> confusing
> >>>>>>>>> since StampedLock does not implement Lock.
> >>>>>>>>>
> >>>>>>>>> Why can't the domain object be null, it's never used by the
> >> framework?
> >>>>>>>>> Why this:
> >>>>>>>>> if (t == lockedObject) {
> >>>>>>>>> throw new IllegalStateException("The returned
> >> object
> >>>>>>>>> is, in fact, the hidden object.");
> >>>>>>>>> }
> >>>>>>>>> ?
> >>>>>>>>> This seems like an application specific constraint that has
> >> nothing to
> >>>>>>> do
> >>>>>>>>> with the framework.
> >>>>>>>>>
> >>>>>>>>> Now that I've considered the above, the API Locks.lock(O) is
> really
> >>>>>>>>> misnamed, because it does not lock anything, it's a factory
> method.
> >>>>>>>>>
> >>>>>>>>> Stepping back even more, since there is only a static inner class
> >> in
> >>>>>>>> Locks,
> >>>>>>>>> and no-hint that alternative implementations for different kind
> of
> >>>>>>> locks
> >>>>>>>>> are possible, I would say we do not need Locks, all we need is
> >> what is
> >>>>>>>> now
> >>>>>>>>> called Lock.
> >>>>>>>>>
> >>>>>>>>> It's not clear why some methods are protected, I would make those
> >>>>>>>> private.
> >>>>>>>>> It's not like I can use or plugin a Lock,
> ReentrantReadWriteLock, a
> >>>>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I
> >> would
> >>>>>>>> then
> >>>>>>>>> make the current Lock class a standalone class which can then be
> >> used
> >>>>>>>> later
> >>>>>>>>> from a factory class (now Locks) where we can then fill in with
> >>>>>>> different
> >>>>>>>>> implementations.
> >>>>>>>>>
> >>>>>>>>> The Javadoc talks about a "hidden" object but it is not hidden
> >> since it
> >>>>>>>> is
> >>>>>>>>> passed to all visitors!
> >>>>>>>>>
> >>>>>>>>> This test assumption is wrong:
> >>>>>>>>>
> >>>>>>>>> If our threads are running concurrently, then we expect to
> be
> >>>>>>>>> faster
> >>>>>>>>> than running one after the other.
> >>>>>>>>>
> >>>>>>>>> The VM or Java spec makes no such guarantee and the tests have no
> >>>>>>> control
> >>>>>>>>> over VM scheduling. There are cases where this will fail when
> under
> >>>>>>> heavy
> >>>>>>>>> load for example where the cost of additional threads becomes
> >>>>>>>> overwhelming.
> >>>>>>>>>
> >>>>>>>>> Another item I am quite doubtful about is hiding checked
> >> exceptions by
> >>>>>>>>> rethrowning them as unchecked. I'd consider this an anti-pattern.
> >> If I
> >>>>>>> am
> >>>>>>>>> using one of our "Failing" interfaces it is because I am
> expecting
> >> it
> >>>>>>> to
> >>>>>>>>> fail. Perhaps this could be made smoother by refactoring to pass
> >> in a
> >>>>>>>>> lambda for an exception handler.
> >>>>>>>>>
> >>>>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc
> needs
> >>>>>>> work):
> >>>>>>>>> https://github.com/apache/commons-lang/pull/559
> >>>>>>>>>
> >>>>>>>>> Not as important as the above:
> >>>>>>>>> The example in the Javadoc uses logging as its domain subject, a
> >>>>>>>> "logging"
> >>>>>>>>> API (PrintStream) which is not a good example IMO. Logging
> >> frameworks
> >>>>>>>>> today like Log4j handle multi-threaded applications normally
> >> without
> >>>>>>>> having
> >>>>>>>>> developers meddle in it. Yes, I understand it's a simple example
> >> but I
> >>>>>>> am
> >>>>>>>>> hoping we can come up with something more realistic or useful.
> >>>>>>>>>
> >>>>>>>>> Thank you,
> >>>>>>>>> Gary
> >>>>>>>>
> >>>>>>>>
> >> ---------------------------------------------------------------------
> >>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Matt Sicker <bo...@gmail.com>
> >>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>
> >>>
> >>>
> >>> --
> >>> Matt Sicker <bo...@gmail.com>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Rob Tompkins <ch...@gmail.com>.
> On Jul 7, 2020, at 6:56 PM, Gary Gregory <ga...@gmail.com> wrote:
>
> In the PR https://github.com/apache/commons-lang/pull/559 I am going with
> "LockingVistors".
I like this name because of its brevity yet clarity.
-Rob
>
> Gary
>
> On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com> wrote:
>
>> I’m not all that familiar with that area of academia. Let me see what I
>> can dig up
>>
>> -Rob
>>
>>> On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> Are there any academic references for this? Or even something from
>>> Java Concurrency in Practice? Those are good sources for names around
>>> concurrency.
>>>
>>> On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
>>>>
>>>> Why not make the name “ThreadedLambdaSynchronizer” or something like
>> that….just something more descriptive here that get’s closer to what the
>> intended functionality is.
>>>>
>>>> That said, I’m not particularly tied to the name
>> “ThreadedLambdaSynchronizer” just spitballing here for something more
>> descriptive than “Lock”
>>>>
>>>> -Rob
>>>>
>>>>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
>>>>>
>>>>> Now that starts to sound like Apache Groovy or Kotlin.
>>>>>
>>>>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com> wrote:
>>>>>>
>>>>>> soemtimes I really wish to rewrite/add some functions in jdk
>> directly...
>>>>>> especially for reusing some package private static functions...
>>>>>>
>>>>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
>>>>>>
>>>>>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
>>>>>>> targeting Java 8. The customers I deal with are migrating from 8 to
>> 11, and
>>>>>>> Java 11 is not everywhere our customers are. So talking about
>> something
>>>>>>> that might end up in Java... 25 seems to be not in our user's best or
>>>>>>> immediate interest. It might be good for Java in the long oh so long
>> term
>>>>>>> of course.
>>>>>>>
>>>>>>> Gary
>>>>>>>
>>>>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com>
>> wrote:
>>>>>>>
>>>>>>>> At first look, I’m a little surprised we’re trying to take on this
>>>>>>>> functionality. Has anyone reached out to the JDK guys to see if
>> they’d be
>>>>>>>> interested in having it in the JDK? That said, if we approach it
>> from
>>>>>>> that
>>>>>>>> path, we would lose the functionality in older versions of java. So
>>>>>>> maybe I
>>>>>>>> just talked myself out of the idea of putting it in the JDK....
>>>>>>>>
>>>>>>>> Just wanted to stream of consciousness my initial gut vibe.
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> -Rob
>>>>>>>>
>>>>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <garydgregory@gmail.com
>>>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi All:
>>>>>>>>>
>>>>>>>>> I know email is a challenging medium for code reviews, so please
>>>>>>> consider
>>>>>>>>> these comments coming from my best intentions, constructive and
>> caring
>>>>>>>> ;-)
>>>>>>>>> Also please excuse the meandering nature of this post.
>>>>>>>>>
>>>>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
>>>>>>> better
>>>>>>>>> names IMO, not just the class name. There is already a JRE
>> interface
>>>>>>>> called
>>>>>>>>> Lock so this class name is confusing (to me.)
>>>>>>>>>
>>>>>>>>> The Javadoc reads in part:
>>>>>>>>>
>>>>>>>>> * This class implements a wrapper for a locked (hidden) object,
>> and
>>>>>>>>> * provides the means to access it. The basic idea, is that the
>> user
>>>>>>>>> * code forsakes all references to the locked object, using only
>> the
>>>>>>>>> * wrapper object, and the accessor methods
>>>>>>>>>
>>>>>>>>> But this is not the case, the object itself is not locked in
>>>>>>>>> the traditional sense with a monitor through a synchronized block,
>> so
>>>>>>> we
>>>>>>>>> need to update the Javadoc as well.
>>>>>>>>>
>>>>>>>>> To me, this is more about executing blocks of code (through
>> lambdas)
>>>>>>>> which
>>>>>>>>> then get 'safe' (via locking) access to an underlying object. This
>>>>>>> tells
>>>>>>>> me
>>>>>>>>> the class is more about the functional interfaces than about a
>> domain
>>>>>>>>> "Object", hence the name "Lock" is not the best. It's really more
>> of a
>>>>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
>>>>>>>>> Instead, maybe:
>>>>>>>>> - StampledLockVisitor (more specific)
>>>>>>>>> - LockingVisitor (more general)
>>>>>>>>> - SafeObject (vague)
>>>>>>>>> - ObjectLocker (vague)
>>>>>>>>> - rejected: LockedObject since the object itself is not locked.
>>>>>>>>> - ?
>>>>>>>>>
>>>>>>>>> What is also confusing IMO is that the instance variable for the
>> object
>>>>>>>> is
>>>>>>>>> called "lockedObject" but the object is in fact NOT locked all the
>>>>>>> time.
>>>>>>>> So
>>>>>>>>> that needs to be renamed IMO:
>>>>>>>>> - object (the simplest)
>>>>>>>>> - subject
>>>>>>>>> - domain
>>>>>>>>> - target
>>>>>>>>> - ?
>>>>>>>>>
>>>>>>>>> In the same vein, the StampedLock is named "lock" which is also
>>>>>>> confusing
>>>>>>>>> since StampedLock does not implement Lock.
>>>>>>>>>
>>>>>>>>> Why can't the domain object be null, it's never used by the
>> framework?
>>>>>>>>> Why this:
>>>>>>>>> if (t == lockedObject) {
>>>>>>>>> throw new IllegalStateException("The returned
>> object
>>>>>>>>> is, in fact, the hidden object.");
>>>>>>>>> }
>>>>>>>>> ?
>>>>>>>>> This seems like an application specific constraint that has
>> nothing to
>>>>>>> do
>>>>>>>>> with the framework.
>>>>>>>>>
>>>>>>>>> Now that I've considered the above, the API Locks.lock(O) is really
>>>>>>>>> misnamed, because it does not lock anything, it's a factory method.
>>>>>>>>>
>>>>>>>>> Stepping back even more, since there is only a static inner class
>> in
>>>>>>>> Locks,
>>>>>>>>> and no-hint that alternative implementations for different kind of
>>>>>>> locks
>>>>>>>>> are possible, I would say we do not need Locks, all we need is
>> what is
>>>>>>>> now
>>>>>>>>> called Lock.
>>>>>>>>>
>>>>>>>>> It's not clear why some methods are protected, I would make those
>>>>>>>> private.
>>>>>>>>> It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
>>>>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I
>> would
>>>>>>>> then
>>>>>>>>> make the current Lock class a standalone class which can then be
>> used
>>>>>>>> later
>>>>>>>>> from a factory class (now Locks) where we can then fill in with
>>>>>>> different
>>>>>>>>> implementations.
>>>>>>>>>
>>>>>>>>> The Javadoc talks about a "hidden" object but it is not hidden
>> since it
>>>>>>>> is
>>>>>>>>> passed to all visitors!
>>>>>>>>>
>>>>>>>>> This test assumption is wrong:
>>>>>>>>>
>>>>>>>>> If our threads are running concurrently, then we expect to be
>>>>>>>>> faster
>>>>>>>>> than running one after the other.
>>>>>>>>>
>>>>>>>>> The VM or Java spec makes no such guarantee and the tests have no
>>>>>>> control
>>>>>>>>> over VM scheduling. There are cases where this will fail when under
>>>>>>> heavy
>>>>>>>>> load for example where the cost of additional threads becomes
>>>>>>>> overwhelming.
>>>>>>>>>
>>>>>>>>> Another item I am quite doubtful about is hiding checked
>> exceptions by
>>>>>>>>> rethrowning them as unchecked. I'd consider this an anti-pattern.
>> If I
>>>>>>> am
>>>>>>>>> using one of our "Failing" interfaces it is because I am expecting
>> it
>>>>>>> to
>>>>>>>>> fail. Perhaps this could be made smoother by refactoring to pass
>> in a
>>>>>>>>> lambda for an exception handler.
>>>>>>>>>
>>>>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc needs
>>>>>>> work):
>>>>>>>>> https://github.com/apache/commons-lang/pull/559
>>>>>>>>>
>>>>>>>>> Not as important as the above:
>>>>>>>>> The example in the Javadoc uses logging as its domain subject, a
>>>>>>>> "logging"
>>>>>>>>> API (PrintStream) which is not a good example IMO. Logging
>> frameworks
>>>>>>>>> today like Log4j handle multi-threaded applications normally
>> without
>>>>>>>> having
>>>>>>>>> developers meddle in it. Yes, I understand it's a simple example
>> but I
>>>>>>> am
>>>>>>>>> hoping we can come up with something more realistic or useful.
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Gary
>>>>>>>>
>>>>>>>>
>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matt Sicker <bo...@gmail.com>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
In the PR https://github.com/apache/commons-lang/pull/559 I am going with
"LockingVistors".
Gary
On Tue, Jul 7, 2020 at 2:33 PM Rob Tompkins <ch...@gmail.com> wrote:
> I’m not all that familiar with that area of academia. Let me see what I
> can dig up
>
> -Rob
>
> > On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
> >
> > Are there any academic references for this? Or even something from
> > Java Concurrency in Practice? Those are good sources for names around
> > concurrency.
> >
> > On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
> >>
> >> Why not make the name “ThreadedLambdaSynchronizer” or something like
> that….just something more descriptive here that get’s closer to what the
> intended functionality is.
> >>
> >> That said, I’m not particularly tied to the name
> “ThreadedLambdaSynchronizer” just spitballing here for something more
> descriptive than “Lock”
> >>
> >> -Rob
> >>
> >>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
> >>>
> >>> Now that starts to sound like Apache Groovy or Kotlin.
> >>>
> >>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com> wrote:
> >>>>
> >>>> soemtimes I really wish to rewrite/add some functions in jdk
> directly...
> >>>> especially for reusing some package private static functions...
> >>>>
> >>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
> >>>>
> >>>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
> >>>>> targeting Java 8. The customers I deal with are migrating from 8 to
> 11, and
> >>>>> Java 11 is not everywhere our customers are. So talking about
> something
> >>>>> that might end up in Java... 25 seems to be not in our user's best or
> >>>>> immediate interest. It might be good for Java in the long oh so long
> term
> >>>>> of course.
> >>>>>
> >>>>> Gary
> >>>>>
> >>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com>
> wrote:
> >>>>>
> >>>>>> At first look, I’m a little surprised we’re trying to take on this
> >>>>>> functionality. Has anyone reached out to the JDK guys to see if
> they’d be
> >>>>>> interested in having it in the JDK? That said, if we approach it
> from
> >>>>> that
> >>>>>> path, we would lose the functionality in older versions of java. So
> >>>>> maybe I
> >>>>>> just talked myself out of the idea of putting it in the JDK....
> >>>>>>
> >>>>>> Just wanted to stream of consciousness my initial gut vibe.
> >>>>>>
> >>>>>> Cheers,
> >>>>>> -Rob
> >>>>>>
> >>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <garydgregory@gmail.com
> >
> >>>>>> wrote:
> >>>>>>>
> >>>>>>> Hi All:
> >>>>>>>
> >>>>>>> I know email is a challenging medium for code reviews, so please
> >>>>> consider
> >>>>>>> these comments coming from my best intentions, constructive and
> caring
> >>>>>> ;-)
> >>>>>>> Also please excuse the meandering nature of this post.
> >>>>>>>
> >>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
> >>>>> better
> >>>>>>> names IMO, not just the class name. There is already a JRE
> interface
> >>>>>> called
> >>>>>>> Lock so this class name is confusing (to me.)
> >>>>>>>
> >>>>>>> The Javadoc reads in part:
> >>>>>>>
> >>>>>>> * This class implements a wrapper for a locked (hidden) object,
> and
> >>>>>>> * provides the means to access it. The basic idea, is that the
> user
> >>>>>>> * code forsakes all references to the locked object, using only
> the
> >>>>>>> * wrapper object, and the accessor methods
> >>>>>>>
> >>>>>>> But this is not the case, the object itself is not locked in
> >>>>>>> the traditional sense with a monitor through a synchronized block,
> so
> >>>>> we
> >>>>>>> need to update the Javadoc as well.
> >>>>>>>
> >>>>>>> To me, this is more about executing blocks of code (through
> lambdas)
> >>>>>> which
> >>>>>>> then get 'safe' (via locking) access to an underlying object. This
> >>>>> tells
> >>>>>> me
> >>>>>>> the class is more about the functional interfaces than about a
> domain
> >>>>>>> "Object", hence the name "Lock" is not the best. It's really more
> of a
> >>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
> >>>>>>> Instead, maybe:
> >>>>>>> - StampledLockVisitor (more specific)
> >>>>>>> - LockingVisitor (more general)
> >>>>>>> - SafeObject (vague)
> >>>>>>> - ObjectLocker (vague)
> >>>>>>> - rejected: LockedObject since the object itself is not locked.
> >>>>>>> - ?
> >>>>>>>
> >>>>>>> What is also confusing IMO is that the instance variable for the
> object
> >>>>>> is
> >>>>>>> called "lockedObject" but the object is in fact NOT locked all the
> >>>>> time.
> >>>>>> So
> >>>>>>> that needs to be renamed IMO:
> >>>>>>> - object (the simplest)
> >>>>>>> - subject
> >>>>>>> - domain
> >>>>>>> - target
> >>>>>>> - ?
> >>>>>>>
> >>>>>>> In the same vein, the StampedLock is named "lock" which is also
> >>>>> confusing
> >>>>>>> since StampedLock does not implement Lock.
> >>>>>>>
> >>>>>>> Why can't the domain object be null, it's never used by the
> framework?
> >>>>>>> Why this:
> >>>>>>> if (t == lockedObject) {
> >>>>>>> throw new IllegalStateException("The returned
> object
> >>>>>>> is, in fact, the hidden object.");
> >>>>>>> }
> >>>>>>> ?
> >>>>>>> This seems like an application specific constraint that has
> nothing to
> >>>>> do
> >>>>>>> with the framework.
> >>>>>>>
> >>>>>>> Now that I've considered the above, the API Locks.lock(O) is really
> >>>>>>> misnamed, because it does not lock anything, it's a factory method.
> >>>>>>>
> >>>>>>> Stepping back even more, since there is only a static inner class
> in
> >>>>>> Locks,
> >>>>>>> and no-hint that alternative implementations for different kind of
> >>>>> locks
> >>>>>>> are possible, I would say we do not need Locks, all we need is
> what is
> >>>>>> now
> >>>>>>> called Lock.
> >>>>>>>
> >>>>>>> It's not clear why some methods are protected, I would make those
> >>>>>> private.
> >>>>>>> It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> >>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I
> would
> >>>>>> then
> >>>>>>> make the current Lock class a standalone class which can then be
> used
> >>>>>> later
> >>>>>>> from a factory class (now Locks) where we can then fill in with
> >>>>> different
> >>>>>>> implementations.
> >>>>>>>
> >>>>>>> The Javadoc talks about a "hidden" object but it is not hidden
> since it
> >>>>>> is
> >>>>>>> passed to all visitors!
> >>>>>>>
> >>>>>>> This test assumption is wrong:
> >>>>>>>
> >>>>>>> If our threads are running concurrently, then we expect to be
> >>>>>>> faster
> >>>>>>> than running one after the other.
> >>>>>>>
> >>>>>>> The VM or Java spec makes no such guarantee and the tests have no
> >>>>> control
> >>>>>>> over VM scheduling. There are cases where this will fail when under
> >>>>> heavy
> >>>>>>> load for example where the cost of additional threads becomes
> >>>>>> overwhelming.
> >>>>>>>
> >>>>>>> Another item I am quite doubtful about is hiding checked
> exceptions by
> >>>>>>> rethrowning them as unchecked. I'd consider this an anti-pattern.
> If I
> >>>>> am
> >>>>>>> using one of our "Failing" interfaces it is because I am expecting
> it
> >>>>> to
> >>>>>>> fail. Perhaps this could be made smoother by refactoring to pass
> in a
> >>>>>>> lambda for an exception handler.
> >>>>>>>
> >>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc needs
> >>>>> work):
> >>>>>>> https://github.com/apache/commons-lang/pull/559
> >>>>>>>
> >>>>>>> Not as important as the above:
> >>>>>>> The example in the Javadoc uses logging as its domain subject, a
> >>>>>> "logging"
> >>>>>>> API (PrintStream) which is not a good example IMO. Logging
> frameworks
> >>>>>>> today like Log4j handle multi-threaded applications normally
> without
> >>>>>> having
> >>>>>>> developers meddle in it. Yes, I understand it's a simple example
> but I
> >>>>> am
> >>>>>>> hoping we can come up with something more realistic or useful.
> >>>>>>>
> >>>>>>> Thank you,
> >>>>>>> Gary
> >>>>>>
> >>>>>>
> ---------------------------------------------------------------------
> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>>
> >>>>>>
> >>>>>
> >>>
> >>>
> >>>
> >>> --
> >>> Matt Sicker <bo...@gmail.com>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >
> >
> > --
> > Matt Sicker <bo...@gmail.com>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Rob Tompkins <ch...@gmail.com>.
I’m not all that familiar with that area of academia. Let me see what I can dig up
-Rob
> On Jul 7, 2020, at 2:19 PM, Matt Sicker <bo...@gmail.com> wrote:
>
> Are there any academic references for this? Or even something from
> Java Concurrency in Practice? Those are good sources for names around
> concurrency.
>
> On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
>>
>> Why not make the name “ThreadedLambdaSynchronizer” or something like that….just something more descriptive here that get’s closer to what the intended functionality is.
>>
>> That said, I’m not particularly tied to the name “ThreadedLambdaSynchronizer” just spitballing here for something more descriptive than “Lock”
>>
>> -Rob
>>
>>> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> Now that starts to sound like Apache Groovy or Kotlin.
>>>
>>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com> wrote:
>>>>
>>>> soemtimes I really wish to rewrite/add some functions in jdk directly...
>>>> especially for reusing some package private static functions...
>>>>
>>>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
>>>>
>>>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
>>>>> targeting Java 8. The customers I deal with are migrating from 8 to 11, and
>>>>> Java 11 is not everywhere our customers are. So talking about something
>>>>> that might end up in Java... 25 seems to be not in our user's best or
>>>>> immediate interest. It might be good for Java in the long oh so long term
>>>>> of course.
>>>>>
>>>>> Gary
>>>>>
>>>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com> wrote:
>>>>>
>>>>>> At first look, I’m a little surprised we’re trying to take on this
>>>>>> functionality. Has anyone reached out to the JDK guys to see if they’d be
>>>>>> interested in having it in the JDK? That said, if we approach it from
>>>>> that
>>>>>> path, we would lose the functionality in older versions of java. So
>>>>> maybe I
>>>>>> just talked myself out of the idea of putting it in the JDK....
>>>>>>
>>>>>> Just wanted to stream of consciousness my initial gut vibe.
>>>>>>
>>>>>> Cheers,
>>>>>> -Rob
>>>>>>
>>>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <ga...@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi All:
>>>>>>>
>>>>>>> I know email is a challenging medium for code reviews, so please
>>>>> consider
>>>>>>> these comments coming from my best intentions, constructive and caring
>>>>>> ;-)
>>>>>>> Also please excuse the meandering nature of this post.
>>>>>>>
>>>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
>>>>> better
>>>>>>> names IMO, not just the class name. There is already a JRE interface
>>>>>> called
>>>>>>> Lock so this class name is confusing (to me.)
>>>>>>>
>>>>>>> The Javadoc reads in part:
>>>>>>>
>>>>>>> * This class implements a wrapper for a locked (hidden) object, and
>>>>>>> * provides the means to access it. The basic idea, is that the user
>>>>>>> * code forsakes all references to the locked object, using only the
>>>>>>> * wrapper object, and the accessor methods
>>>>>>>
>>>>>>> But this is not the case, the object itself is not locked in
>>>>>>> the traditional sense with a monitor through a synchronized block, so
>>>>> we
>>>>>>> need to update the Javadoc as well.
>>>>>>>
>>>>>>> To me, this is more about executing blocks of code (through lambdas)
>>>>>> which
>>>>>>> then get 'safe' (via locking) access to an underlying object. This
>>>>> tells
>>>>>> me
>>>>>>> the class is more about the functional interfaces than about a domain
>>>>>>> "Object", hence the name "Lock" is not the best. It's really more of a
>>>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
>>>>>>> Instead, maybe:
>>>>>>> - StampledLockVisitor (more specific)
>>>>>>> - LockingVisitor (more general)
>>>>>>> - SafeObject (vague)
>>>>>>> - ObjectLocker (vague)
>>>>>>> - rejected: LockedObject since the object itself is not locked.
>>>>>>> - ?
>>>>>>>
>>>>>>> What is also confusing IMO is that the instance variable for the object
>>>>>> is
>>>>>>> called "lockedObject" but the object is in fact NOT locked all the
>>>>> time.
>>>>>> So
>>>>>>> that needs to be renamed IMO:
>>>>>>> - object (the simplest)
>>>>>>> - subject
>>>>>>> - domain
>>>>>>> - target
>>>>>>> - ?
>>>>>>>
>>>>>>> In the same vein, the StampedLock is named "lock" which is also
>>>>> confusing
>>>>>>> since StampedLock does not implement Lock.
>>>>>>>
>>>>>>> Why can't the domain object be null, it's never used by the framework?
>>>>>>> Why this:
>>>>>>> if (t == lockedObject) {
>>>>>>> throw new IllegalStateException("The returned object
>>>>>>> is, in fact, the hidden object.");
>>>>>>> }
>>>>>>> ?
>>>>>>> This seems like an application specific constraint that has nothing to
>>>>> do
>>>>>>> with the framework.
>>>>>>>
>>>>>>> Now that I've considered the above, the API Locks.lock(O) is really
>>>>>>> misnamed, because it does not lock anything, it's a factory method.
>>>>>>>
>>>>>>> Stepping back even more, since there is only a static inner class in
>>>>>> Locks,
>>>>>>> and no-hint that alternative implementations for different kind of
>>>>> locks
>>>>>>> are possible, I would say we do not need Locks, all we need is what is
>>>>>> now
>>>>>>> called Lock.
>>>>>>>
>>>>>>> It's not clear why some methods are protected, I would make those
>>>>>> private.
>>>>>>> It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
>>>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would
>>>>>> then
>>>>>>> make the current Lock class a standalone class which can then be used
>>>>>> later
>>>>>>> from a factory class (now Locks) where we can then fill in with
>>>>> different
>>>>>>> implementations.
>>>>>>>
>>>>>>> The Javadoc talks about a "hidden" object but it is not hidden since it
>>>>>> is
>>>>>>> passed to all visitors!
>>>>>>>
>>>>>>> This test assumption is wrong:
>>>>>>>
>>>>>>> If our threads are running concurrently, then we expect to be
>>>>>>> faster
>>>>>>> than running one after the other.
>>>>>>>
>>>>>>> The VM or Java spec makes no such guarantee and the tests have no
>>>>> control
>>>>>>> over VM scheduling. There are cases where this will fail when under
>>>>> heavy
>>>>>>> load for example where the cost of additional threads becomes
>>>>>> overwhelming.
>>>>>>>
>>>>>>> Another item I am quite doubtful about is hiding checked exceptions by
>>>>>>> rethrowning them as unchecked. I'd consider this an anti-pattern. If I
>>>>> am
>>>>>>> using one of our "Failing" interfaces it is because I am expecting it
>>>>> to
>>>>>>> fail. Perhaps this could be made smoother by refactoring to pass in a
>>>>>>> lambda for an exception handler.
>>>>>>>
>>>>>>> I've crystallized my thoughts into code here as WIP (Javadoc needs
>>>>> work):
>>>>>>> https://github.com/apache/commons-lang/pull/559
>>>>>>>
>>>>>>> Not as important as the above:
>>>>>>> The example in the Javadoc uses logging as its domain subject, a
>>>>>> "logging"
>>>>>>> API (PrintStream) which is not a good example IMO. Logging frameworks
>>>>>>> today like Log4j handle multi-threaded applications normally without
>>>>>> having
>>>>>>> developers meddle in it. Yes, I understand it's a simple example but I
>>>>> am
>>>>>>> hoping we can come up with something more realistic or useful.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Gary
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Matt Sicker <bo...@gmail.com>.
Are there any academic references for this? Or even something from
Java Concurrency in Practice? Those are good sources for names around
concurrency.
On Tue, 7 Jul 2020 at 12:42, Rob Tompkins <ch...@gmail.com> wrote:
>
> Why not make the name “ThreadedLambdaSynchronizer” or something like that….just something more descriptive here that get’s closer to what the intended functionality is.
>
> That said, I’m not particularly tied to the name “ThreadedLambdaSynchronizer” just spitballing here for something more descriptive than “Lock”
>
> -Rob
>
> > On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
> >
> > Now that starts to sound like Apache Groovy or Kotlin.
> >
> > On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com> wrote:
> >>
> >> soemtimes I really wish to rewrite/add some functions in jdk directly...
> >> especially for reusing some package private static functions...
> >>
> >> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
> >>
> >>> I'm not sure talking to the JDK folks is helpful IMO. We are still
> >>> targeting Java 8. The customers I deal with are migrating from 8 to 11, and
> >>> Java 11 is not everywhere our customers are. So talking about something
> >>> that might end up in Java... 25 seems to be not in our user's best or
> >>> immediate interest. It might be good for Java in the long oh so long term
> >>> of course.
> >>>
> >>> Gary
> >>>
> >>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com> wrote:
> >>>
> >>>> At first look, I’m a little surprised we’re trying to take on this
> >>>> functionality. Has anyone reached out to the JDK guys to see if they’d be
> >>>> interested in having it in the JDK? That said, if we approach it from
> >>> that
> >>>> path, we would lose the functionality in older versions of java. So
> >>> maybe I
> >>>> just talked myself out of the idea of putting it in the JDK....
> >>>>
> >>>> Just wanted to stream of consciousness my initial gut vibe.
> >>>>
> >>>> Cheers,
> >>>> -Rob
> >>>>
> >>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <ga...@gmail.com>
> >>>> wrote:
> >>>>>
> >>>>> Hi All:
> >>>>>
> >>>>> I know email is a challenging medium for code reviews, so please
> >>> consider
> >>>>> these comments coming from my best intentions, constructive and caring
> >>>> ;-)
> >>>>> Also please excuse the meandering nature of this post.
> >>>>>
> >>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
> >>> better
> >>>>> names IMO, not just the class name. There is already a JRE interface
> >>>> called
> >>>>> Lock so this class name is confusing (to me.)
> >>>>>
> >>>>> The Javadoc reads in part:
> >>>>>
> >>>>> * This class implements a wrapper for a locked (hidden) object, and
> >>>>> * provides the means to access it. The basic idea, is that the user
> >>>>> * code forsakes all references to the locked object, using only the
> >>>>> * wrapper object, and the accessor methods
> >>>>>
> >>>>> But this is not the case, the object itself is not locked in
> >>>>> the traditional sense with a monitor through a synchronized block, so
> >>> we
> >>>>> need to update the Javadoc as well.
> >>>>>
> >>>>> To me, this is more about executing blocks of code (through lambdas)
> >>>> which
> >>>>> then get 'safe' (via locking) access to an underlying object. This
> >>> tells
> >>>> me
> >>>>> the class is more about the functional interfaces than about a domain
> >>>>> "Object", hence the name "Lock" is not the best. It's really more of a
> >>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
> >>>>> Instead, maybe:
> >>>>> - StampledLockVisitor (more specific)
> >>>>> - LockingVisitor (more general)
> >>>>> - SafeObject (vague)
> >>>>> - ObjectLocker (vague)
> >>>>> - rejected: LockedObject since the object itself is not locked.
> >>>>> - ?
> >>>>>
> >>>>> What is also confusing IMO is that the instance variable for the object
> >>>> is
> >>>>> called "lockedObject" but the object is in fact NOT locked all the
> >>> time.
> >>>> So
> >>>>> that needs to be renamed IMO:
> >>>>> - object (the simplest)
> >>>>> - subject
> >>>>> - domain
> >>>>> - target
> >>>>> - ?
> >>>>>
> >>>>> In the same vein, the StampedLock is named "lock" which is also
> >>> confusing
> >>>>> since StampedLock does not implement Lock.
> >>>>>
> >>>>> Why can't the domain object be null, it's never used by the framework?
> >>>>> Why this:
> >>>>> if (t == lockedObject) {
> >>>>> throw new IllegalStateException("The returned object
> >>>>> is, in fact, the hidden object.");
> >>>>> }
> >>>>> ?
> >>>>> This seems like an application specific constraint that has nothing to
> >>> do
> >>>>> with the framework.
> >>>>>
> >>>>> Now that I've considered the above, the API Locks.lock(O) is really
> >>>>> misnamed, because it does not lock anything, it's a factory method.
> >>>>>
> >>>>> Stepping back even more, since there is only a static inner class in
> >>>> Locks,
> >>>>> and no-hint that alternative implementations for different kind of
> >>> locks
> >>>>> are possible, I would say we do not need Locks, all we need is what is
> >>>> now
> >>>>> called Lock.
> >>>>>
> >>>>> It's not clear why some methods are protected, I would make those
> >>>> private.
> >>>>> It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> >>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would
> >>>> then
> >>>>> make the current Lock class a standalone class which can then be used
> >>>> later
> >>>>> from a factory class (now Locks) where we can then fill in with
> >>> different
> >>>>> implementations.
> >>>>>
> >>>>> The Javadoc talks about a "hidden" object but it is not hidden since it
> >>>> is
> >>>>> passed to all visitors!
> >>>>>
> >>>>> This test assumption is wrong:
> >>>>>
> >>>>> If our threads are running concurrently, then we expect to be
> >>>>> faster
> >>>>> than running one after the other.
> >>>>>
> >>>>> The VM or Java spec makes no such guarantee and the tests have no
> >>> control
> >>>>> over VM scheduling. There are cases where this will fail when under
> >>> heavy
> >>>>> load for example where the cost of additional threads becomes
> >>>> overwhelming.
> >>>>>
> >>>>> Another item I am quite doubtful about is hiding checked exceptions by
> >>>>> rethrowning them as unchecked. I'd consider this an anti-pattern. If I
> >>> am
> >>>>> using one of our "Failing" interfaces it is because I am expecting it
> >>> to
> >>>>> fail. Perhaps this could be made smoother by refactoring to pass in a
> >>>>> lambda for an exception handler.
> >>>>>
> >>>>> I've crystallized my thoughts into code here as WIP (Javadoc needs
> >>> work):
> >>>>> https://github.com/apache/commons-lang/pull/559
> >>>>>
> >>>>> Not as important as the above:
> >>>>> The example in the Javadoc uses logging as its domain subject, a
> >>>> "logging"
> >>>>> API (PrintStream) which is not a good example IMO. Logging frameworks
> >>>>> today like Log4j handle multi-threaded applications normally without
> >>>> having
> >>>>> developers meddle in it. Yes, I understand it's a simple example but I
> >>> am
> >>>>> hoping we can come up with something more realistic or useful.
> >>>>>
> >>>>> Thank you,
> >>>>> Gary
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>
> >>>>
> >>>
> >
> >
> >
> > --
> > Matt Sicker <bo...@gmail.com>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
--
Matt Sicker <bo...@gmail.com>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Rob Tompkins <ch...@gmail.com>.
Why not make the name “ThreadedLambdaSynchronizer” or something like that….just something more descriptive here that get’s closer to what the intended functionality is.
That said, I’m not particularly tied to the name “ThreadedLambdaSynchronizer” just spitballing here for something more descriptive than “Lock”
-Rob
> On Jun 29, 2020, at 12:58 PM, Matt Sicker <bo...@gmail.com> wrote:
>
> Now that starts to sound like Apache Groovy or Kotlin.
>
> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com> wrote:
>>
>> soemtimes I really wish to rewrite/add some functions in jdk directly...
>> especially for reusing some package private static functions...
>>
>> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
>>
>>> I'm not sure talking to the JDK folks is helpful IMO. We are still
>>> targeting Java 8. The customers I deal with are migrating from 8 to 11, and
>>> Java 11 is not everywhere our customers are. So talking about something
>>> that might end up in Java... 25 seems to be not in our user's best or
>>> immediate interest. It might be good for Java in the long oh so long term
>>> of course.
>>>
>>> Gary
>>>
>>> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com> wrote:
>>>
>>>> At first look, I’m a little surprised we’re trying to take on this
>>>> functionality. Has anyone reached out to the JDK guys to see if they’d be
>>>> interested in having it in the JDK? That said, if we approach it from
>>> that
>>>> path, we would lose the functionality in older versions of java. So
>>> maybe I
>>>> just talked myself out of the idea of putting it in the JDK....
>>>>
>>>> Just wanted to stream of consciousness my initial gut vibe.
>>>>
>>>> Cheers,
>>>> -Rob
>>>>
>>>>> On Jun 26, 2020, at 10:07 AM, Gary Gregory <ga...@gmail.com>
>>>> wrote:
>>>>>
>>>>> Hi All:
>>>>>
>>>>> I know email is a challenging medium for code reviews, so please
>>> consider
>>>>> these comments coming from my best intentions, constructive and caring
>>>> ;-)
>>>>> Also please excuse the meandering nature of this post.
>>>>>
>>>>> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
>>> better
>>>>> names IMO, not just the class name. There is already a JRE interface
>>>> called
>>>>> Lock so this class name is confusing (to me.)
>>>>>
>>>>> The Javadoc reads in part:
>>>>>
>>>>> * This class implements a wrapper for a locked (hidden) object, and
>>>>> * provides the means to access it. The basic idea, is that the user
>>>>> * code forsakes all references to the locked object, using only the
>>>>> * wrapper object, and the accessor methods
>>>>>
>>>>> But this is not the case, the object itself is not locked in
>>>>> the traditional sense with a monitor through a synchronized block, so
>>> we
>>>>> need to update the Javadoc as well.
>>>>>
>>>>> To me, this is more about executing blocks of code (through lambdas)
>>>> which
>>>>> then get 'safe' (via locking) access to an underlying object. This
>>> tells
>>>> me
>>>>> the class is more about the functional interfaces than about a domain
>>>>> "Object", hence the name "Lock" is not the best. It's really more of a
>>>>> visitor where the visitation pattern is: Lock, Visit, Unlock.
>>>>> Instead, maybe:
>>>>> - StampledLockVisitor (more specific)
>>>>> - LockingVisitor (more general)
>>>>> - SafeObject (vague)
>>>>> - ObjectLocker (vague)
>>>>> - rejected: LockedObject since the object itself is not locked.
>>>>> - ?
>>>>>
>>>>> What is also confusing IMO is that the instance variable for the object
>>>> is
>>>>> called "lockedObject" but the object is in fact NOT locked all the
>>> time.
>>>> So
>>>>> that needs to be renamed IMO:
>>>>> - object (the simplest)
>>>>> - subject
>>>>> - domain
>>>>> - target
>>>>> - ?
>>>>>
>>>>> In the same vein, the StampedLock is named "lock" which is also
>>> confusing
>>>>> since StampedLock does not implement Lock.
>>>>>
>>>>> Why can't the domain object be null, it's never used by the framework?
>>>>> Why this:
>>>>> if (t == lockedObject) {
>>>>> throw new IllegalStateException("The returned object
>>>>> is, in fact, the hidden object.");
>>>>> }
>>>>> ?
>>>>> This seems like an application specific constraint that has nothing to
>>> do
>>>>> with the framework.
>>>>>
>>>>> Now that I've considered the above, the API Locks.lock(O) is really
>>>>> misnamed, because it does not lock anything, it's a factory method.
>>>>>
>>>>> Stepping back even more, since there is only a static inner class in
>>>> Locks,
>>>>> and no-hint that alternative implementations for different kind of
>>> locks
>>>>> are possible, I would say we do not need Locks, all we need is what is
>>>> now
>>>>> called Lock.
>>>>>
>>>>> It's not clear why some methods are protected, I would make those
>>>> private.
>>>>> It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
>>>>> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would
>>>> then
>>>>> make the current Lock class a standalone class which can then be used
>>>> later
>>>>> from a factory class (now Locks) where we can then fill in with
>>> different
>>>>> implementations.
>>>>>
>>>>> The Javadoc talks about a "hidden" object but it is not hidden since it
>>>> is
>>>>> passed to all visitors!
>>>>>
>>>>> This test assumption is wrong:
>>>>>
>>>>> If our threads are running concurrently, then we expect to be
>>>>> faster
>>>>> than running one after the other.
>>>>>
>>>>> The VM or Java spec makes no such guarantee and the tests have no
>>> control
>>>>> over VM scheduling. There are cases where this will fail when under
>>> heavy
>>>>> load for example where the cost of additional threads becomes
>>>> overwhelming.
>>>>>
>>>>> Another item I am quite doubtful about is hiding checked exceptions by
>>>>> rethrowning them as unchecked. I'd consider this an anti-pattern. If I
>>> am
>>>>> using one of our "Failing" interfaces it is because I am expecting it
>>> to
>>>>> fail. Perhaps this could be made smoother by refactoring to pass in a
>>>>> lambda for an exception handler.
>>>>>
>>>>> I've crystallized my thoughts into code here as WIP (Javadoc needs
>>> work):
>>>>> https://github.com/apache/commons-lang/pull/559
>>>>>
>>>>> Not as important as the above:
>>>>> The example in the Javadoc uses logging as its domain subject, a
>>>> "logging"
>>>>> API (PrintStream) which is not a good example IMO. Logging frameworks
>>>>> today like Log4j handle multi-threaded applications normally without
>>>> having
>>>>> developers meddle in it. Yes, I understand it's a simple example but I
>>> am
>>>>> hoping we can come up with something more realistic or useful.
>>>>>
>>>>> Thank you,
>>>>> Gary
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Matt Sicker <bo...@gmail.com>.
Now that starts to sound like Apache Groovy or Kotlin.
On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xe...@gmail.com> wrote:
>
> soemtimes I really wish to rewrite/add some functions in jdk directly...
> especially for reusing some package private static functions...
>
> Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
>
> > I'm not sure talking to the JDK folks is helpful IMO. We are still
> > targeting Java 8. The customers I deal with are migrating from 8 to 11, and
> > Java 11 is not everywhere our customers are. So talking about something
> > that might end up in Java... 25 seems to be not in our user's best or
> > immediate interest. It might be good for Java in the long oh so long term
> > of course.
> >
> > Gary
> >
> > On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com> wrote:
> >
> > > At first look, I’m a little surprised we’re trying to take on this
> > > functionality. Has anyone reached out to the JDK guys to see if they’d be
> > > interested in having it in the JDK? That said, if we approach it from
> > that
> > > path, we would lose the functionality in older versions of java. So
> > maybe I
> > > just talked myself out of the idea of putting it in the JDK....
> > >
> > > Just wanted to stream of consciousness my initial gut vibe.
> > >
> > > Cheers,
> > > -Rob
> > >
> > > > On Jun 26, 2020, at 10:07 AM, Gary Gregory <ga...@gmail.com>
> > > wrote:
> > > >
> > > > Hi All:
> > > >
> > > > I know email is a challenging medium for code reviews, so please
> > consider
> > > > these comments coming from my best intentions, constructive and caring
> > > ;-)
> > > > Also please excuse the meandering nature of this post.
> > > >
> > > > The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
> > better
> > > > names IMO, not just the class name. There is already a JRE interface
> > > called
> > > > Lock so this class name is confusing (to me.)
> > > >
> > > > The Javadoc reads in part:
> > > >
> > > > * This class implements a wrapper for a locked (hidden) object, and
> > > > * provides the means to access it. The basic idea, is that the user
> > > > * code forsakes all references to the locked object, using only the
> > > > * wrapper object, and the accessor methods
> > > >
> > > > But this is not the case, the object itself is not locked in
> > > > the traditional sense with a monitor through a synchronized block, so
> > we
> > > > need to update the Javadoc as well.
> > > >
> > > > To me, this is more about executing blocks of code (through lambdas)
> > > which
> > > > then get 'safe' (via locking) access to an underlying object. This
> > tells
> > > me
> > > > the class is more about the functional interfaces than about a domain
> > > > "Object", hence the name "Lock" is not the best. It's really more of a
> > > > visitor where the visitation pattern is: Lock, Visit, Unlock.
> > > > Instead, maybe:
> > > > - StampledLockVisitor (more specific)
> > > > - LockingVisitor (more general)
> > > > - SafeObject (vague)
> > > > - ObjectLocker (vague)
> > > > - rejected: LockedObject since the object itself is not locked.
> > > > - ?
> > > >
> > > > What is also confusing IMO is that the instance variable for the object
> > > is
> > > > called "lockedObject" but the object is in fact NOT locked all the
> > time.
> > > So
> > > > that needs to be renamed IMO:
> > > > - object (the simplest)
> > > > - subject
> > > > - domain
> > > > - target
> > > > - ?
> > > >
> > > > In the same vein, the StampedLock is named "lock" which is also
> > confusing
> > > > since StampedLock does not implement Lock.
> > > >
> > > > Why can't the domain object be null, it's never used by the framework?
> > > > Why this:
> > > > if (t == lockedObject) {
> > > > throw new IllegalStateException("The returned object
> > > > is, in fact, the hidden object.");
> > > > }
> > > > ?
> > > > This seems like an application specific constraint that has nothing to
> > do
> > > > with the framework.
> > > >
> > > > Now that I've considered the above, the API Locks.lock(O) is really
> > > > misnamed, because it does not lock anything, it's a factory method.
> > > >
> > > > Stepping back even more, since there is only a static inner class in
> > > Locks,
> > > > and no-hint that alternative implementations for different kind of
> > locks
> > > > are possible, I would say we do not need Locks, all we need is what is
> > > now
> > > > called Lock.
> > > >
> > > > It's not clear why some methods are protected, I would make those
> > > private.
> > > > It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> > > > ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would
> > > then
> > > > make the current Lock class a standalone class which can then be used
> > > later
> > > > from a factory class (now Locks) where we can then fill in with
> > different
> > > > implementations.
> > > >
> > > > The Javadoc talks about a "hidden" object but it is not hidden since it
> > > is
> > > > passed to all visitors!
> > > >
> > > > This test assumption is wrong:
> > > >
> > > > If our threads are running concurrently, then we expect to be
> > > > faster
> > > > than running one after the other.
> > > >
> > > > The VM or Java spec makes no such guarantee and the tests have no
> > control
> > > > over VM scheduling. There are cases where this will fail when under
> > heavy
> > > > load for example where the cost of additional threads becomes
> > > overwhelming.
> > > >
> > > > Another item I am quite doubtful about is hiding checked exceptions by
> > > > rethrowning them as unchecked. I'd consider this an anti-pattern. If I
> > am
> > > > using one of our "Failing" interfaces it is because I am expecting it
> > to
> > > > fail. Perhaps this could be made smoother by refactoring to pass in a
> > > > lambda for an exception handler.
> > > >
> > > > I've crystallized my thoughts into code here as WIP (Javadoc needs
> > work):
> > > > https://github.com/apache/commons-lang/pull/559
> > > >
> > > > Not as important as the above:
> > > > The example in the Javadoc uses logging as its domain subject, a
> > > "logging"
> > > > API (PrintStream) which is not a good example IMO. Logging frameworks
> > > > today like Log4j handle multi-threaded applications normally without
> > > having
> > > > developers meddle in it. Yes, I understand it's a simple example but I
> > am
> > > > hoping we can come up with something more realistic or useful.
> > > >
> > > > Thank you,
> > > > Gary
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >
> > >
> >
--
Matt Sicker <bo...@gmail.com>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Xeno Amess <xe...@gmail.com>.
soemtimes I really wish to rewrite/add some functions in jdk directly...
especially for reusing some package private static functions...
Gary Gregory <ga...@gmail.com> 于2020年6月30日周二 上午12:01写道：
> I'm not sure talking to the JDK folks is helpful IMO. We are still
> targeting Java 8. The customers I deal with are migrating from 8 to 11, and
> Java 11 is not everywhere our customers are. So talking about something
> that might end up in Java... 25 seems to be not in our user's best or
> immediate interest. It might be good for Java in the long oh so long term
> of course.
>
> Gary
>
> On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com> wrote:
>
> > At first look, I’m a little surprised we’re trying to take on this
> > functionality. Has anyone reached out to the JDK guys to see if they’d be
> > interested in having it in the JDK? That said, if we approach it from
> that
> > path, we would lose the functionality in older versions of java. So
> maybe I
> > just talked myself out of the idea of putting it in the JDK....
> >
> > Just wanted to stream of consciousness my initial gut vibe.
> >
> > Cheers,
> > -Rob
> >
> > > On Jun 26, 2020, at 10:07 AM, Gary Gregory <ga...@gmail.com>
> > wrote:
> > >
> > > Hi All:
> > >
> > > I know email is a challenging medium for code reviews, so please
> consider
> > > these comments coming from my best intentions, constructive and caring
> > ;-)
> > > Also please excuse the meandering nature of this post.
> > >
> > > The new class org.apache.commons.lang3.concurrent.Locks.Lock needs
> better
> > > names IMO, not just the class name. There is already a JRE interface
> > called
> > > Lock so this class name is confusing (to me.)
> > >
> > > The Javadoc reads in part:
> > >
> > > * This class implements a wrapper for a locked (hidden) object, and
> > > * provides the means to access it. The basic idea, is that the user
> > > * code forsakes all references to the locked object, using only the
> > > * wrapper object, and the accessor methods
> > >
> > > But this is not the case, the object itself is not locked in
> > > the traditional sense with a monitor through a synchronized block, so
> we
> > > need to update the Javadoc as well.
> > >
> > > To me, this is more about executing blocks of code (through lambdas)
> > which
> > > then get 'safe' (via locking) access to an underlying object. This
> tells
> > me
> > > the class is more about the functional interfaces than about a domain
> > > "Object", hence the name "Lock" is not the best. It's really more of a
> > > visitor where the visitation pattern is: Lock, Visit, Unlock.
> > > Instead, maybe:
> > > - StampledLockVisitor (more specific)
> > > - LockingVisitor (more general)
> > > - SafeObject (vague)
> > > - ObjectLocker (vague)
> > > - rejected: LockedObject since the object itself is not locked.
> > > - ?
> > >
> > > What is also confusing IMO is that the instance variable for the object
> > is
> > > called "lockedObject" but the object is in fact NOT locked all the
> time.
> > So
> > > that needs to be renamed IMO:
> > > - object (the simplest)
> > > - subject
> > > - domain
> > > - target
> > > - ?
> > >
> > > In the same vein, the StampedLock is named "lock" which is also
> confusing
> > > since StampedLock does not implement Lock.
> > >
> > > Why can't the domain object be null, it's never used by the framework?
> > > Why this:
> > > if (t == lockedObject) {
> > > throw new IllegalStateException("The returned object
> > > is, in fact, the hidden object.");
> > > }
> > > ?
> > > This seems like an application specific constraint that has nothing to
> do
> > > with the framework.
> > >
> > > Now that I've considered the above, the API Locks.lock(O) is really
> > > misnamed, because it does not lock anything, it's a factory method.
> > >
> > > Stepping back even more, since there is only a static inner class in
> > Locks,
> > > and no-hint that alternative implementations for different kind of
> locks
> > > are possible, I would say we do not need Locks, all we need is what is
> > now
> > > called Lock.
> > >
> > > It's not clear why some methods are protected, I would make those
> > private.
> > > It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> > > ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would
> > then
> > > make the current Lock class a standalone class which can then be used
> > later
> > > from a factory class (now Locks) where we can then fill in with
> different
> > > implementations.
> > >
> > > The Javadoc talks about a "hidden" object but it is not hidden since it
> > is
> > > passed to all visitors!
> > >
> > > This test assumption is wrong:
> > >
> > > If our threads are running concurrently, then we expect to be
> > > faster
> > > than running one after the other.
> > >
> > > The VM or Java spec makes no such guarantee and the tests have no
> control
> > > over VM scheduling. There are cases where this will fail when under
> heavy
> > > load for example where the cost of additional threads becomes
> > overwhelming.
> > >
> > > Another item I am quite doubtful about is hiding checked exceptions by
> > > rethrowning them as unchecked. I'd consider this an anti-pattern. If I
> am
> > > using one of our "Failing" interfaces it is because I am expecting it
> to
> > > fail. Perhaps this could be made smoother by refactoring to pass in a
> > > lambda for an exception handler.
> > >
> > > I've crystallized my thoughts into code here as WIP (Javadoc needs
> work):
> > > https://github.com/apache/commons-lang/pull/559
> > >
> > > Not as important as the above:
> > > The example in the Javadoc uses logging as its domain subject, a
> > "logging"
> > > API (PrintStream) which is not a good example IMO. Logging frameworks
> > > today like Log4j handle multi-threaded applications normally without
> > having
> > > developers meddle in it. Yes, I understand it's a simple example but I
> am
> > > hoping we can come up with something more realistic or useful.
> > >
> > > Thank you,
> > > Gary
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
I'm not sure talking to the JDK folks is helpful IMO. We are still
targeting Java 8. The customers I deal with are migrating from 8 to 11, and
Java 11 is not everywhere our customers are. So talking about something
that might end up in Java... 25 seems to be not in our user's best or
immediate interest. It might be good for Java in the long oh so long term
of course.
Gary
On Mon, Jun 29, 2020 at 8:31 AM Rob Tompkins <ch...@gmail.com> wrote:
> At first look, I’m a little surprised we’re trying to take on this
> functionality. Has anyone reached out to the JDK guys to see if they’d be
> interested in having it in the JDK? That said, if we approach it from that
> path, we would lose the functionality in older versions of java. So maybe I
> just talked myself out of the idea of putting it in the JDK....
>
> Just wanted to stream of consciousness my initial gut vibe.
>
> Cheers,
> -Rob
>
> > On Jun 26, 2020, at 10:07 AM, Gary Gregory <ga...@gmail.com>
> wrote:
> >
> > Hi All:
> >
> > I know email is a challenging medium for code reviews, so please consider
> > these comments coming from my best intentions, constructive and caring
> ;-)
> > Also please excuse the meandering nature of this post.
> >
> > The new class org.apache.commons.lang3.concurrent.Locks.Lock needs better
> > names IMO, not just the class name. There is already a JRE interface
> called
> > Lock so this class name is confusing (to me.)
> >
> > The Javadoc reads in part:
> >
> > * This class implements a wrapper for a locked (hidden) object, and
> > * provides the means to access it. The basic idea, is that the user
> > * code forsakes all references to the locked object, using only the
> > * wrapper object, and the accessor methods
> >
> > But this is not the case, the object itself is not locked in
> > the traditional sense with a monitor through a synchronized block, so we
> > need to update the Javadoc as well.
> >
> > To me, this is more about executing blocks of code (through lambdas)
> which
> > then get 'safe' (via locking) access to an underlying object. This tells
> me
> > the class is more about the functional interfaces than about a domain
> > "Object", hence the name "Lock" is not the best. It's really more of a
> > visitor where the visitation pattern is: Lock, Visit, Unlock.
> > Instead, maybe:
> > - StampledLockVisitor (more specific)
> > - LockingVisitor (more general)
> > - SafeObject (vague)
> > - ObjectLocker (vague)
> > - rejected: LockedObject since the object itself is not locked.
> > - ?
> >
> > What is also confusing IMO is that the instance variable for the object
> is
> > called "lockedObject" but the object is in fact NOT locked all the time.
> So
> > that needs to be renamed IMO:
> > - object (the simplest)
> > - subject
> > - domain
> > - target
> > - ?
> >
> > In the same vein, the StampedLock is named "lock" which is also confusing
> > since StampedLock does not implement Lock.
> >
> > Why can't the domain object be null, it's never used by the framework?
> > Why this:
> > if (t == lockedObject) {
> > throw new IllegalStateException("The returned object
> > is, in fact, the hidden object.");
> > }
> > ?
> > This seems like an application specific constraint that has nothing to do
> > with the framework.
> >
> > Now that I've considered the above, the API Locks.lock(O) is really
> > misnamed, because it does not lock anything, it's a factory method.
> >
> > Stepping back even more, since there is only a static inner class in
> Locks,
> > and no-hint that alternative implementations for different kind of locks
> > are possible, I would say we do not need Locks, all we need is what is
> now
> > called Lock.
> >
> > It's not clear why some methods are protected, I would make those
> private.
> > It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> > ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would
> then
> > make the current Lock class a standalone class which can then be used
> later
> > from a factory class (now Locks) where we can then fill in with different
> > implementations.
> >
> > The Javadoc talks about a "hidden" object but it is not hidden since it
> is
> > passed to all visitors!
> >
> > This test assumption is wrong:
> >
> > If our threads are running concurrently, then we expect to be
> > faster
> > than running one after the other.
> >
> > The VM or Java spec makes no such guarantee and the tests have no control
> > over VM scheduling. There are cases where this will fail when under heavy
> > load for example where the cost of additional threads becomes
> overwhelming.
> >
> > Another item I am quite doubtful about is hiding checked exceptions by
> > rethrowning them as unchecked. I'd consider this an anti-pattern. If I am
> > using one of our "Failing" interfaces it is because I am expecting it to
> > fail. Perhaps this could be made smoother by refactoring to pass in a
> > lambda for an exception handler.
> >
> > I've crystallized my thoughts into code here as WIP (Javadoc needs work):
> > https://github.com/apache/commons-lang/pull/559
> >
> > Not as important as the above:
> > The example in the Javadoc uses logging as its domain subject, a
> "logging"
> > API (PrintStream) which is not a good example IMO. Logging frameworks
> > today like Log4j handle multi-threaded applications normally without
> having
> > developers meddle in it. Yes, I understand it's a simple example but I am
> > hoping we can come up with something more realistic or useful.
> >
> > Thank you,
> > Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Rob Tompkins <ch...@gmail.com>.
At first look, I’m a little surprised we’re trying to take on this functionality. Has anyone reached out to the JDK guys to see if they’d be interested in having it in the JDK? That said, if we approach it from that path, we would lose the functionality in older versions of java. So maybe I just talked myself out of the idea of putting it in the JDK....
Just wanted to stream of consciousness my initial gut vibe.
Cheers,
-Rob
> On Jun 26, 2020, at 10:07 AM, Gary Gregory <ga...@gmail.com> wrote:
>
> Hi All:
>
> I know email is a challenging medium for code reviews, so please consider
> these comments coming from my best intentions, constructive and caring ;-)
> Also please excuse the meandering nature of this post.
>
> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs better
> names IMO, not just the class name. There is already a JRE interface called
> Lock so this class name is confusing (to me.)
>
> The Javadoc reads in part:
>
> * This class implements a wrapper for a locked (hidden) object, and
> * provides the means to access it. The basic idea, is that the user
> * code forsakes all references to the locked object, using only the
> * wrapper object, and the accessor methods
>
> But this is not the case, the object itself is not locked in
> the traditional sense with a monitor through a synchronized block, so we
> need to update the Javadoc as well.
>
> To me, this is more about executing blocks of code (through lambdas) which
> then get 'safe' (via locking) access to an underlying object. This tells me
> the class is more about the functional interfaces than about a domain
> "Object", hence the name "Lock" is not the best. It's really more of a
> visitor where the visitation pattern is: Lock, Visit, Unlock.
> Instead, maybe:
> - StampledLockVisitor (more specific)
> - LockingVisitor (more general)
> - SafeObject (vague)
> - ObjectLocker (vague)
> - rejected: LockedObject since the object itself is not locked.
> - ?
>
> What is also confusing IMO is that the instance variable for the object is
> called "lockedObject" but the object is in fact NOT locked all the time. So
> that needs to be renamed IMO:
> - object (the simplest)
> - subject
> - domain
> - target
> - ?
>
> In the same vein, the StampedLock is named "lock" which is also confusing
> since StampedLock does not implement Lock.
>
> Why can't the domain object be null, it's never used by the framework?
> Why this:
> if (t == lockedObject) {
> throw new IllegalStateException("The returned object
> is, in fact, the hidden object.");
> }
> ?
> This seems like an application specific constraint that has nothing to do
> with the framework.
>
> Now that I've considered the above, the API Locks.lock(O) is really
> misnamed, because it does not lock anything, it's a factory method.
>
> Stepping back even more, since there is only a static inner class in Locks,
> and no-hint that alternative implementations for different kind of locks
> are possible, I would say we do not need Locks, all we need is what is now
> called Lock.
>
> It's not clear why some methods are protected, I would make those private.
> It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would then
> make the current Lock class a standalone class which can then be used later
> from a factory class (now Locks) where we can then fill in with different
> implementations.
>
> The Javadoc talks about a "hidden" object but it is not hidden since it is
> passed to all visitors!
>
> This test assumption is wrong:
>
> If our threads are running concurrently, then we expect to be
> faster
> than running one after the other.
>
> The VM or Java spec makes no such guarantee and the tests have no control
> over VM scheduling. There are cases where this will fail when under heavy
> load for example where the cost of additional threads becomes overwhelming.
>
> Another item I am quite doubtful about is hiding checked exceptions by
> rethrowning them as unchecked. I'd consider this an anti-pattern. If I am
> using one of our "Failing" interfaces it is because I am expecting it to
> fail. Perhaps this could be made smoother by refactoring to pass in a
> lambda for an exception handler.
>
> I've crystallized my thoughts into code here as WIP (Javadoc needs work):
> https://github.com/apache/commons-lang/pull/559
>
> Not as important as the above:
> The example in the Javadoc uses logging as its domain subject, a "logging"
> API (PrintStream) which is not a good example IMO. Logging frameworks
> today like Log4j handle multi-threaded applications normally without having
> developers meddle in it. Yes, I understand it's a simple example but I am
> hoping we can come up with something more realistic or useful.
>
> Thank you,
> Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Gary Gregory <ga...@gmail.com>.
The 'Locks' class is in master. My PR is in my 1st message.
Gary
On Fri, Jun 26, 2020 at 5:27 PM Miguel Muñoz <sw...@gmail.com> wrote:
> The "lockedObject" might be better named "lockableObject." That way it
> makes no assumptions about its current state. Building on this, the class
> might be called Lockable.
>
> However, I also like the idea of calling the class Locker, as in a gym
> locker. The idea is that it holds something important. Or (since Locker
> also sounds like a verb form) maybe it could be called a LockBox. That
> implies that it wraps something, which is what we want.
>
> I may come up with better suggestions if I could see the class. It's not in
> master, and I don't see it on any branch. Can you provide a link?
>
> — Miguel Muñoz
>
> On Fri, Jun 26, 2020 at 7:07 AM Gary Gregory <ga...@gmail.com>
> wrote:
> >
> > Hi All:
> >
> > I know email is a challenging medium for code reviews, so please consider
> > these comments coming from my best intentions, constructive and caring
> ;-)
> > Also please excuse the meandering nature of this post.
> >
> > The new class org.apache.commons.lang3.concurrent.Locks.Lock needs better
> > names IMO, not just the class name. There is already a JRE interface
> called
> > Lock so this class name is confusing (to me.)
> >
> > The Javadoc reads in part:
> >
> > * This class implements a wrapper for a locked (hidden) object, and
> > * provides the means to access it. The basic idea, is that the user
> > * code forsakes all references to the locked object, using only the
> > * wrapper object, and the accessor methods
> >
> > But this is not the case, the object itself is not locked in
> > the traditional sense with a monitor through a synchronized block, so we
> > need to update the Javadoc as well.
> >
> > To me, this is more about executing blocks of code (through lambdas)
> which
> > then get 'safe' (via locking) access to an underlying object. This tells
> me
> > the class is more about the functional interfaces than about a domain
> > "Object", hence the name "Lock" is not the best. It's really more of a
> > visitor where the visitation pattern is: Lock, Visit, Unlock.
> > Instead, maybe:
> > - StampledLockVisitor (more specific)
> > - LockingVisitor (more general)
> > - SafeObject (vague)
> > - ObjectLocker (vague)
> > - rejected: LockedObject since the object itself is not locked.
> > - ?
> >
> > What is also confusing IMO is that the instance variable for the object
> is
> > called "lockedObject" but the object is in fact NOT locked all the time.
> So
> > that needs to be renamed IMO:
> > - object (the simplest)
> > - subject
> > - domain
> > - target
> > - ?
> >
> > In the same vein, the StampedLock is named "lock" which is also confusing
> > since StampedLock does not implement Lock.
> >
> > Why can't the domain object be null, it's never used by the framework?
> > Why this:
> > if (t == lockedObject) {
> > throw new IllegalStateException("The returned object
> > is, in fact, the hidden object.");
> > }
> > ?
> > This seems like an application specific constraint that has nothing to do
> > with the framework.
> >
> > Now that I've considered the above, the API Locks.lock(O) is really
> > misnamed, because it does not lock anything, it's a factory method.
> >
> > Stepping back even more, since there is only a static inner class in
> Locks,
> > and no-hint that alternative implementations for different kind of locks
> > are possible, I would say we do not need Locks, all we need is what is
> now
> > called Lock.
> >
> > It's not clear why some methods are protected, I would make those
> private.
> > It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> > ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would
> then
> > make the current Lock class a standalone class which can then be used
> later
> > from a factory class (now Locks) where we can then fill in with different
> > implementations.
> >
> > The Javadoc talks about a "hidden" object but it is not hidden since it
> is
> > passed to all visitors!
> >
> > This test assumption is wrong:
> >
> > If our threads are running concurrently, then we expect to be
> > faster
> > than running one after the other.
> >
> > The VM or Java spec makes no such guarantee and the tests have no control
> > over VM scheduling. There are cases where this will fail when under heavy
> > load for example where the cost of additional threads becomes
> overwhelming.
> >
> > Another item I am quite doubtful about is hiding checked exceptions by
> > rethrowning them as unchecked. I'd consider this an anti-pattern. If I am
> > using one of our "Failing" interfaces it is because I am expecting it to
> > fail. Perhaps this could be made smoother by refactoring to pass in a
> > lambda for an exception handler.
> >
> > I've crystallized my thoughts into code here as WIP (Javadoc needs work):
> > https://github.com/apache/commons-lang/pull/559
> >
> > Not as important as the above:
> > The example in the Javadoc uses logging as its domain subject, a
> "logging"
> > API (PrintStream) which is not a good example IMO. Logging frameworks
> > today like Log4j handle multi-threaded applications normally without
> having
> > developers meddle in it. Yes, I understand it's a simple example but I am
> > hoping we can come up with something more realistic or useful.
> >
> > Thank you,
> > Gary
>
Re: [lang] org.apache.commons.lang3.concurrent.Locks.Lock
Posted by Miguel Muñoz <sw...@gmail.com>.
The "lockedObject" might be better named "lockableObject." That way it
makes no assumptions about its current state. Building on this, the class
might be called Lockable.
However, I also like the idea of calling the class Locker, as in a gym
locker. The idea is that it holds something important. Or (since Locker
also sounds like a verb form) maybe it could be called a LockBox. That
implies that it wraps something, which is what we want.
I may come up with better suggestions if I could see the class. It's not in
master, and I don't see it on any branch. Can you provide a link?
— Miguel Muñoz
On Fri, Jun 26, 2020 at 7:07 AM Gary Gregory <ga...@gmail.com> wrote:
>
> Hi All:
>
> I know email is a challenging medium for code reviews, so please consider
> these comments coming from my best intentions, constructive and caring ;-)
> Also please excuse the meandering nature of this post.
>
> The new class org.apache.commons.lang3.concurrent.Locks.Lock needs better
> names IMO, not just the class name. There is already a JRE interface
called
> Lock so this class name is confusing (to me.)
>
> The Javadoc reads in part:
>
> * This class implements a wrapper for a locked (hidden) object, and
> * provides the means to access it. The basic idea, is that the user
> * code forsakes all references to the locked object, using only the
> * wrapper object, and the accessor methods
>
> But this is not the case, the object itself is not locked in
> the traditional sense with a monitor through a synchronized block, so we
> need to update the Javadoc as well.
>
> To me, this is more about executing blocks of code (through lambdas) which
> then get 'safe' (via locking) access to an underlying object. This tells
me
> the class is more about the functional interfaces than about a domain
> "Object", hence the name "Lock" is not the best. It's really more of a
> visitor where the visitation pattern is: Lock, Visit, Unlock.
> Instead, maybe:
> - StampledLockVisitor (more specific)
> - LockingVisitor (more general)
> - SafeObject (vague)
> - ObjectLocker (vague)
> - rejected: LockedObject since the object itself is not locked.
> - ?
>
> What is also confusing IMO is that the instance variable for the object is
> called "lockedObject" but the object is in fact NOT locked all the time.
So
> that needs to be renamed IMO:
> - object (the simplest)
> - subject
> - domain
> - target
> - ?
>
> In the same vein, the StampedLock is named "lock" which is also confusing
> since StampedLock does not implement Lock.
>
> Why can't the domain object be null, it's never used by the framework?
> Why this:
> if (t == lockedObject) {
> throw new IllegalStateException("The returned object
> is, in fact, the hidden object.");
> }
> ?
> This seems like an application specific constraint that has nothing to do
> with the framework.
>
> Now that I've considered the above, the API Locks.lock(O) is really
> misnamed, because it does not lock anything, it's a factory method.
>
> Stepping back even more, since there is only a static inner class in
Locks,
> and no-hint that alternative implementations for different kind of locks
> are possible, I would say we do not need Locks, all we need is what is now
> called Lock.
>
> It's not clear why some methods are protected, I would make those private.
> It's not like I can use or plugin a Lock, ReentrantReadWriteLock, a
> ReentrantLock, or any ReadWriteLock instead of a StampedLock. I would then
> make the current Lock class a standalone class which can then be used
later
> from a factory class (now Locks) where we can then fill in with different
> implementations.
>
> The Javadoc talks about a "hidden" object but it is not hidden since it is
> passed to all visitors!
>
> This test assumption is wrong:
>
> If our threads are running concurrently, then we expect to be
> faster
> than running one after the other.
>
> The VM or Java spec makes no such guarantee and the tests have no control
> over VM scheduling. There are cases where this will fail when under heavy
> load for example where the cost of additional threads becomes
overwhelming.
>
> Another item I am quite doubtful about is hiding checked exceptions by
> rethrowning them as unchecked. I'd consider this an anti-pattern. If I am
> using one of our "Failing" interfaces it is because I am expecting it to
> fail. Perhaps this could be made smoother by refactoring to pass in a
> lambda for an exception handler.
>
> I've crystallized my thoughts into code here as WIP (Javadoc needs work):
> https://github.com/apache/commons-lang/pull/559
>
> Not as important as the above:
> The example in the Javadoc uses logging as its domain subject, a "logging"
> API (PrintStream) which is not a good example IMO. Logging frameworks
> today like Log4j handle multi-threaded applications normally without
having
> developers meddle in it. Yes, I understand it's a simple example but I am
> hoping we can come up with something more realistic or useful.
>
> Thank you,
> Gary