TimurMahammadov / google-collections

Automatically exported from code.google.com/p/google-collections
Apache License 2.0
0 stars 0 forks source link

BiMap interface lacks appropriate methods for concurrent use #28

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
For example, putIfAbsent().

Original issue reported on code.google.com by kevin...@gmail.com on 31 Oct 2007 at 8:40

GoogleCodeExporter commented 9 years ago
You sure these should go in the base interface?  Or should there be a 
ConcurrentBiMap
interface?

Original comment by jahlborn@gmail.com on 1 Nov 2007 at 2:14

GoogleCodeExporter commented 9 years ago
If these went in a subinterface, it would be essential *not* to name in
ConcurrentBiMap.  All we have to do is look at the Map/ConcurrentMap disaster.  
Those
methods would absolutely have gone into Map if it were possible to do so, and 
they
would have gone into an intermediate interface which did not imply actual
concurrent/atomic implementation, if only Doug could have come up with a name 
for
that interface (granted, I can't either. nor can I figure out what an 
intermediate
BiMap interface would be called).

I'm not sure I see the benefit of the separate interface given that we'd want 
all our
BiMap implementations to implement it. So then it becomes just a separation of 
the
"basic methods" from the "advanced methods" or something.

It may be that the methods we need to add to BiMap are precisely the same ones 
as are
in ConcurrentMap, and that the other types of atomic operations we could 
envision
could be performed simply by using those methods on the inverse view.  Or there 
could
be others.

Original comment by kevin...@gmail.com on 1 Nov 2007 at 5:28

GoogleCodeExporter commented 9 years ago
I'm not sure i follow the purpose of them being a non-concurrent API.  all can 
be
easily implemented with the current Map API.  is it because they involve 
multiple
lookups which can be optimized to one lookup for an internal implementation?

Original comment by jahlborn@gmail.com on 1 Nov 2007 at 6:59

GoogleCodeExporter commented 9 years ago
A couple reasons:

1. Because these methods are missing from Map, the 
Collections.synchronizedMap() view
is less useful than otherwise; clients must always take care to wrap atomic 
sections
in their own explicit synchronization.

2. I should be able to write a method that modifies a Map without having to 
know or
care whether it is a thread-confined/thread-safe regular Map or whether it is a
ConcurrentMap.  Right now I can't do that.  I must use an instanceof check and 
do
things one way in one case, another way in the other.

Original comment by kevin...@gmail.com on 1 Nov 2007 at 7:29

GoogleCodeExporter commented 9 years ago
well, 1 is certainly a valid point.  Of course, Collections.synchronizedMap 
could
always be changed to return a ConcurrentMap.  in fact, that could be added to 
this
library!

on 2, though, i think i have to disagree.  only in the most trivial case is 
this an
issue (where your method calls exactly one method on the map).  if you are 
doing more
than one logical operation on a map, you still need to be aware of the 
underlying
implementation or you will not get the results you expect.

Original comment by jahlborn@gmail.com on 1 Nov 2007 at 7:46

GoogleCodeExporter commented 9 years ago
Not necessarily; it's perfectly conceivable that my method would make a series 
of
changes which are individually atomic, and don't break any of my invariants. 
(tangent: this is how ConcurrentMap.putAll() works. it's not atomic, but it 
doesn't
have to be, because it comprises multiple atomic operations that always 
preserve the
invariants.)

But if one of my individually-atomic changes I want to make is a putIfAbsent(),
here's what I have to do:

  if (map instanceof ConcurrentMap<?, ?>) {
    ConcurrentMap<Foo, Bar> cmap = (ConcurrentMap<Foo, Bar>) map;
    cmap.putIfAbsent(aFoo, aBar);
  } else {
    synchronized (map) { // just in case? 
      return map.containsKey(aFoo) ? map.get(aFoo) : map.put(aFoo, aBar);
    }
  }

or something like that.

Original comment by kevin...@gmail.com on 1 Nov 2007 at 9:48

GoogleCodeExporter commented 9 years ago
Must decide what to do, whatever that may be, and do it for 1.0.

Original comment by kevin...@gmail.com on 27 May 2008 at 6:32

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 27 May 2008 at 7:19