On Wed, Jul 8, 2020, 09:32 Rob Tompkins <chtom...@gmail.com> wrote:
>
>
> > On Jul 7, 2020, at 6:56 PM, Gary Gregory <garydgreg...@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 <chtom...@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 <boa...@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 <chtom...@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 <boa...@gmail.com> wrote:
> >>>>>
> >>>>> Now that starts to sound like Apache Groovy or Kotlin.
> >>>>>
> >>>>> On Mon, 29 Jun 2020 at 11:58, Xeno Amess <xenoam...@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 <garydgreg...@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 <chtom...@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 <
> garydgreg...@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-unsubscr...@commons.apache.org
> >>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Matt Sicker <boa...@gmail.com>
> >>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>>
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>
> >>>
> >>>
> >>> --
> >>> Matt Sicker <boa...@gmail.com>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >> For additional commands, e-mail: dev-h...@commons.apache.org
> >>
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
