river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dan Creswell <dan.cresw...@gmail.com>
Subject Re: MarshalledServiceItem
Date Tue, 08 Feb 2011 16:26:26 GMT
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 <gregg@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<gregg@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.creswell@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<gregg@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<jini@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();
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message