On Tue, Jul 7, 2020 at 10:22 PM Gary Gregory <garydgreg...@gmail.com> wrote:
> On Tue, Jul 7, 2020 at 7:14 PM Matt Sicker <boa...@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 <garydgreg...@gmail.com> wrote: >> >> > 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 >> > >> >> > >> >> > >> -- >> Matt Sicker <boa...@gmail.com> >> >
