Typo: LockingVisitors. On Tue, 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". > > 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 >> >>