On Wed, Jul 8, 2020 at 10:13 AM Gary Gregory <garydgreg...@gmail.com> wrote:
> > > 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. > 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 <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 >> >>
