No friction, simplest thing that could possibly work is the only thing running through my head.
I'm happy to see CHM used if people deem it appropriate - as is so often the case, I'm not writing the code so my opinion should only carry so much weight. On 8 February 2011 15:12, Gregg Wonderly <gr...@wonderly.org> wrote: > On 2/8/2011 9:00 AM, Dan Creswell wrote: > >> It's all about expected usage patterns. >> >> Locking in this case will be per instance and the likelihood of contention >> is low given no one's going to call add a lot or indeed toString a lot in >> a >> short space of time. >> >> So given: >> >> Define how concurrency is handled then implement >> >> And assuming it's in line with the above contention model I'd say >> synchronized would be adequate. >> > > I believe this is the case. But, I also know that it can come back to > bite. > > CHM solves it without synchronized, and that, I believe is a better choice > because it eliminates the problem. Any additional methods or usage patterns > that fall out of the life cycle of this object will not have any new or > different behavior or synchronization requirements. > > Is there an attribute of CHM that you find unattractive Dan? I'm not sure > I understand the friction I am sensing. > > Gregg > > > On 8 February 2011 14:54, Gregg Wonderly<gr...@wonderly.org> wrote: >> >> On 2/7/2011 5:28 PM, Peter Firmstone wrote: >>> >>> Do we really need a concurrent version of this class? How often will >>>> Entry >>>> classes change? >>>> >>>> >>> My choice is always to consider concurrency first, unless there is some >>> element of performance that this impacts. The simple fact is that there >>> is >>> a toString() that traverses the contents. So, you either must >>> synchronize >>> all access to the contents, or use a CHM which solves the problem without >>> any future concurrency surprises. >>> >>> I'm really serious about this, because of how I've encountered tons of >>> "synchronized" blocks in river which introduce huge latency in >>> "processing" >>> because they cause everyone to wait in line for stuff which very often is >>> quite independent. >>> >>> Yes, this structure is mostly one time mutated. But, its design exposes >>> a >>> continuous mutation capability. Thus, any "read" of the contents has to >>> take that into account. >>> >>> Gregg Wonderly >>> >>> >>> Gregg Wonderly wrote: >>> >>>> >>>> Yes definitely the case. CHM just makes it a lot easier to not worry >>>>> about map >>>>> and set data structures in terms of concurrent access of mutating data. >>>>> Set >>>>> and HashSet are useful, but not always in concurrent mutating data >>>>> management. >>>>> >>>>> I still prefer Vector over ArrayList just because it eliminates >>>>> iteration >>>>> problems that alway require the same lines of code to resolve. >>>>> >>>>> Gregg >>>>> >>>>> Sent from my iPhone >>>>> >>>>> On Feb 7, 2011, at 2:08 PM, Dan Creswell<dan.cresw...@gmail.com> >>>>> wrote: >>>>> >>>>> All true, still need to document the concurrent contract for this >>>>> class >>>>> >>>>>> and >>>>>> implement accordingly. >>>>>> >>>>>> i.e. We can say "toString" is thread safe etc and then use >>>>>> ConcurrentHashMap >>>>>> and friends. >>>>>> >>>>>> But we gotta do it right.... >>>>>> >>>>>> On 7 February 2011 19:57, Gregg Wonderly<gr...@wonderly.org> wrote: >>>>>> >>>>>> On 2/7/2011 5:24 AM, Tom Hobbs wrote: >>>>>> >>>>>>> >>>>>>> Sorry, I've got my nit-picking hat on... >>>>>>> >>>>>>>> >>>>>>>> The null check in the equals method isn't needed because you get a >>>>>>>> free one by using "instanceof". >>>>>>>> >>>>>>>> I know you've warned that the class is not thread-safe, but people >>>>>>>> often forget that doing a sysout of an object calls toString(). I'd >>>>>>>> still be tempted to whack some protection into toString to stop it >>>>>>>> blowing up when people start printing lots of debug messages. >>>>>>>> >>>>>>>> Other than that it looks good to me. >>>>>>>> >>>>>>>> I'd suggest that it would be easy enough to just use >>>>>>>> >>>>>>> ConcurrentHashMap >>>>>>> instead of HashSet if we are going to move to Java 5 anyway. Then, >>>>>>> you >>>>>>> don't have to worry about concurrency issues with toString() etc. >>>>>>> >>>>>>> Gregg Wonderly >>>>>>> >>>>>>> >>>>>>> On Mon, Feb 7, 2011 at 10:56 AM, Peter Firmstone<j...@zeus.net.au> >>>>>>> >>>>>>> wrote: >>>>>>>> >>>>>>>> Dan Creswell wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> Solid - probably ought to be a constructor that doesn't add >>>>>>>>> defaults >>>>>>>>> >>>>>>>>>> too.....and maybe a note about thread safety on add. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> The finished article. >>>>>>>>>> >>>>>>>>> >>>>>>>>> /* >>>>>>>>> * Licensed to the Apache Software Foundation (ASF) under one >>>>>>>>> * or more contributor license agreements. See the NOTICE file >>>>>>>>> * distributed with this work for additional information >>>>>>>>> * regarding copyright ownership. The ASF licenses this file >>>>>>>>> * to you under the Apache License, Version 2.0 (the >>>>>>>>> * "License"); you may not use this file except in compliance >>>>>>>>> * with the License. You may obtain a copy of the License at >>>>>>>>> * >>>>>>>>> * http://www.apache.org/licenses/LICENSE-2.0 >>>>>>>>> * >>>>>>>>> * Unless required by applicable law or agreed to in writing, >>>>>>>>> software >>>>>>>>> * distributed under the License is distributed on an "AS IS" BASIS, >>>>>>>>> * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >>>>>>>>> implied. >>>>>>>>> * See the License for the specific language governing permissions >>>>>>>>> and >>>>>>>>> * limitations under the License. >>>>>>>>> */ >>>>>>>>> >>>>>>>>> package org.apache.river.api.lookup; >>>>>>>>> >>>>>>>>> import java.util.HashSet; >>>>>>>>> import java.util.Iterator; >>>>>>>>> import java.util.Set; >>>>>>>>> import net.jini.lookup.entry.Address; >>>>>>>>> import net.jini.lookup.entry.Comment; >>>>>>>>> import net.jini.lookup.entry.Location; >>>>>>>>> import net.jini.lookup.entry.Name; >>>>>>>>> import net.jini.lookup.entry.ServiceInfo; >>>>>>>>> import net.jini.lookup.entry.Status; >>>>>>>>> import net.jini.lookup.entry.UIDescriptor; >>>>>>>>> >>>>>>>>> /** >>>>>>>>> * A little builder utility class that creates an array of Entry >>>>>>>>> classes >>>>>>>>> to >>>>>>>>> * be used as a parameter for StreamServiceRegistrar. All the jini >>>>>>>>> platform >>>>>>>>> * Entry's are included by default. >>>>>>>>> * >>>>>>>>> * Note: This class is not threadsafe, use external synchronization >>>>>>>>> if >>>>>>>>> required. >>>>>>>>> * >>>>>>>>> * Suggested by Dan Creswell. >>>>>>>>> * @author peter >>>>>>>>> */ >>>>>>>>> public class DefaultEntries { >>>>>>>>> private final Set<Class> entrys; >>>>>>>>> public DefaultEntries() { >>>>>>>>> entrys = new HashSet<Class>(16); >>>>>>>>> } >>>>>>>>> /** >>>>>>>>> * Add an Entry class. >>>>>>>>> * @param cl - class >>>>>>>>> * @return this >>>>>>>>> */ >>>>>>>>> public DefaultEntries add(Class cl){ >>>>>>>>> entrys.add(cl); >>>>>>>>> return this; >>>>>>>>> } >>>>>>>>> /** >>>>>>>>> * All all the Jini Platform Entry's >>>>>>>>> * @return >>>>>>>>> */ >>>>>>>>> public DefaultEntries addPlatformEntries(){ >>>>>>>>> add(Comment.class); >>>>>>>>> add(Location.class); >>>>>>>>> add(Name.class); >>>>>>>>> add(ServiceInfo.class); >>>>>>>>> add(Status.class); >>>>>>>>> add(UIDescriptor.class); >>>>>>>>> add(Address.class); >>>>>>>>> return this; >>>>>>>>> } >>>>>>>>> /** >>>>>>>>> * Remove all Entry's >>>>>>>>> */ >>>>>>>>> public void reset(){ >>>>>>>>> entrys.clear(); >>>>>>>>> } >>>>>>>>> /** >>>>>>>>> * Generate a new array containing all Entry's added since last >>>>>>>>> reset. >>>>>>>>> * @return >>>>>>>>> */ >>>>>>>>> public Class[] getEntries(){ >>>>>>>>> return entrys.toArray(new Class[entrys.size()]); >>>>>>>>> } >>>>>>>>> >>>>>>>>> @Override >>>>>>>>> public int hashCode() { >>>>>>>>> int hash = 3; >>>>>>>>> hash = 29 * hash + (this.entrys != null ? this.entrys.hashCode() : >>>>>>>>> 0); >>>>>>>>> return hash; >>>>>>>>> } >>>>>>>>> >>>>>>>>> @Override >>>>>>>>> public boolean equals(Object o){ >>>>>>>>> if (o == null) return false; >>>>>>>>> if (o instanceof DefaultEntries){ >>>>>>>>> if (entrys.equals(((DefaultEntries)o).entrys)) return true; >>>>>>>>> } >>>>>>>>> return false; >>>>>>>>> } >>>>>>>>> @Override >>>>>>>>> public String toString(){ >>>>>>>>> String newline = System.getProperty("line.separator"); >>>>>>>>> StringBuilder sb = new StringBuilder(256); >>>>>>>>> sb.append("DefaultEntries:"); >>>>>>>>> sb.append(newline); >>>>>>>>> Iterator<Class> it = entrys.iterator(); >>>>>>>>> while (it.hasNext()){ >>>>>>>>> sb.append(it.next().getName()); >>>>>>>>> sb.append(newline); >>>>>>>>> } >>>>>>>>> return sb.toString(); >>>>>>>>> } >>>>>>>>> } >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>> >>>> >>>> >>> >> >
