Yuta HIGUCHI

GossipLinkStore: switch inner Map to non-Concurrent

Change-Id: I7489cef4cb40c2201a5be3435b63fc5ccd87e033
...@@ -9,7 +9,6 @@ import com.google.common.collect.Maps; ...@@ -9,7 +9,6 @@ import com.google.common.collect.Maps;
9 import com.google.common.collect.SetMultimap; 9 import com.google.common.collect.SetMultimap;
10 10
11 import org.apache.commons.lang3.RandomUtils; 11 import org.apache.commons.lang3.RandomUtils;
12 -import org.apache.commons.lang3.concurrent.ConcurrentUtils;
13 import org.apache.felix.scr.annotations.Activate; 12 import org.apache.felix.scr.annotations.Activate;
14 import org.apache.felix.scr.annotations.Component; 13 import org.apache.felix.scr.annotations.Component;
15 import org.apache.felix.scr.annotations.Deactivate; 14 import org.apache.felix.scr.annotations.Deactivate;
...@@ -46,7 +45,6 @@ import org.onlab.onos.store.common.impl.Timestamped; ...@@ -46,7 +45,6 @@ import org.onlab.onos.store.common.impl.Timestamped;
46 import org.onlab.onos.store.serializers.DistributedStoreSerializers; 45 import org.onlab.onos.store.serializers.DistributedStoreSerializers;
47 import org.onlab.onos.store.serializers.KryoSerializer; 46 import org.onlab.onos.store.serializers.KryoSerializer;
48 import org.onlab.util.KryoPool; 47 import org.onlab.util.KryoPool;
49 -import org.onlab.util.NewConcurrentHashMap;
50 import org.slf4j.Logger; 48 import org.slf4j.Logger;
51 49
52 import java.io.IOException; 50 import java.io.IOException;
...@@ -87,7 +85,7 @@ public class GossipLinkStore ...@@ -87,7 +85,7 @@ public class GossipLinkStore
87 private final Logger log = getLogger(getClass()); 85 private final Logger log = getLogger(getClass());
88 86
89 // Link inventory 87 // Link inventory
90 - private final ConcurrentMap<LinkKey, ConcurrentMap<ProviderId, Timestamped<LinkDescription>>> linkDescs = 88 + private final ConcurrentMap<LinkKey, Map<ProviderId, Timestamped<LinkDescription>>> linkDescs =
91 new ConcurrentHashMap<>(); 89 new ConcurrentHashMap<>();
92 90
93 // Link instance cache 91 // Link instance cache
...@@ -267,7 +265,7 @@ public class GossipLinkStore ...@@ -267,7 +265,7 @@ public class GossipLinkStore
267 265
268 LinkKey key = linkKey(linkDescription.value().src(), 266 LinkKey key = linkKey(linkDescription.value().src(),
269 linkDescription.value().dst()); 267 linkDescription.value().dst());
270 - ConcurrentMap<ProviderId, Timestamped<LinkDescription>> descs = getLinkDescriptions(key); 268 + Map<ProviderId, Timestamped<LinkDescription>> descs = getLinkDescriptions(key);
271 269
272 synchronized (descs) { 270 synchronized (descs) {
273 // if the link was previously removed, we should proceed if and 271 // if the link was previously removed, we should proceed if and
...@@ -294,12 +292,12 @@ public class GossipLinkStore ...@@ -294,12 +292,12 @@ public class GossipLinkStore
294 292
295 // Guarded by linkDescs value (=locking each Link) 293 // Guarded by linkDescs value (=locking each Link)
296 private Timestamped<LinkDescription> createOrUpdateLinkDescription( 294 private Timestamped<LinkDescription> createOrUpdateLinkDescription(
297 - ConcurrentMap<ProviderId, Timestamped<LinkDescription>> existingLinkDescriptions, 295 + Map<ProviderId, Timestamped<LinkDescription>> descs,
298 ProviderId providerId, 296 ProviderId providerId,
299 Timestamped<LinkDescription> linkDescription) { 297 Timestamped<LinkDescription> linkDescription) {
300 298
301 // merge existing attributes and merge 299 // merge existing attributes and merge
302 - Timestamped<LinkDescription> existingLinkDescription = existingLinkDescriptions.get(providerId); 300 + Timestamped<LinkDescription> existingLinkDescription = descs.get(providerId);
303 if (existingLinkDescription != null && existingLinkDescription.isNewer(linkDescription)) { 301 if (existingLinkDescription != null && existingLinkDescription.isNewer(linkDescription)) {
304 return null; 302 return null;
305 } 303 }
...@@ -314,7 +312,7 @@ public class GossipLinkStore ...@@ -314,7 +312,7 @@ public class GossipLinkStore
314 linkDescription.value().type(), merged), 312 linkDescription.value().type(), merged),
315 linkDescription.timestamp()); 313 linkDescription.timestamp());
316 } 314 }
317 - return existingLinkDescriptions.put(providerId, newLinkDescription); 315 + return descs.put(providerId, newLinkDescription);
318 } 316 }
319 317
320 // Creates and stores the link and returns the appropriate event. 318 // Creates and stores the link and returns the appropriate event.
...@@ -380,7 +378,7 @@ public class GossipLinkStore ...@@ -380,7 +378,7 @@ public class GossipLinkStore
380 } 378 }
381 379
382 private LinkEvent removeLinkInternal(LinkKey key, Timestamp timestamp) { 380 private LinkEvent removeLinkInternal(LinkKey key, Timestamp timestamp) {
383 - ConcurrentMap<ProviderId, Timestamped<LinkDescription>> linkDescriptions = 381 + Map<ProviderId, Timestamped<LinkDescription>> linkDescriptions =
384 getLinkDescriptions(key); 382 getLinkDescriptions(key);
385 synchronized (linkDescriptions) { 383 synchronized (linkDescriptions) {
386 // accept removal request if given timestamp is newer than 384 // accept removal request if given timestamp is newer than
...@@ -409,10 +407,10 @@ public class GossipLinkStore ...@@ -409,10 +407,10 @@ public class GossipLinkStore
409 * @return primary ProviderID, or randomly chosen one if none exists 407 * @return primary ProviderID, or randomly chosen one if none exists
410 */ 408 */
411 private ProviderId pickPrimaryProviderId( 409 private ProviderId pickPrimaryProviderId(
412 - ConcurrentMap<ProviderId, Timestamped<LinkDescription>> providerDescs) { 410 + Map<ProviderId, Timestamped<LinkDescription>> linkDescriptions) {
413 411
414 ProviderId fallBackPrimary = null; 412 ProviderId fallBackPrimary = null;
415 - for (Entry<ProviderId, Timestamped<LinkDescription>> e : providerDescs.entrySet()) { 413 + for (Entry<ProviderId, Timestamped<LinkDescription>> e : linkDescriptions.entrySet()) {
416 if (!e.getKey().isAncillary()) { 414 if (!e.getKey().isAncillary()) {
417 return e.getKey(); 415 return e.getKey();
418 } else if (fallBackPrimary == null) { 416 } else if (fallBackPrimary == null) {
...@@ -423,9 +421,9 @@ public class GossipLinkStore ...@@ -423,9 +421,9 @@ public class GossipLinkStore
423 return fallBackPrimary; 421 return fallBackPrimary;
424 } 422 }
425 423
426 - private Link composeLink(ConcurrentMap<ProviderId, Timestamped<LinkDescription>> linkDescriptions) { 424 + private Link composeLink(Map<ProviderId, Timestamped<LinkDescription>> descs) {
427 - ProviderId primaryProviderId = pickPrimaryProviderId(linkDescriptions); 425 + ProviderId primaryProviderId = pickPrimaryProviderId(descs);
428 - Timestamped<LinkDescription> base = linkDescriptions.get(primaryProviderId); 426 + Timestamped<LinkDescription> base = descs.get(primaryProviderId);
429 427
430 ConnectPoint src = base.value().src(); 428 ConnectPoint src = base.value().src();
431 ConnectPoint dst = base.value().dst(); 429 ConnectPoint dst = base.value().dst();
...@@ -433,7 +431,7 @@ public class GossipLinkStore ...@@ -433,7 +431,7 @@ public class GossipLinkStore
433 DefaultAnnotations annotations = DefaultAnnotations.builder().build(); 431 DefaultAnnotations annotations = DefaultAnnotations.builder().build();
434 annotations = merge(annotations, base.value().annotations()); 432 annotations = merge(annotations, base.value().annotations());
435 433
436 - for (Entry<ProviderId, Timestamped<LinkDescription>> e : linkDescriptions.entrySet()) { 434 + for (Entry<ProviderId, Timestamped<LinkDescription>> e : descs.entrySet()) {
437 if (primaryProviderId.equals(e.getKey())) { 435 if (primaryProviderId.equals(e.getKey())) {
438 continue; 436 continue;
439 } 437 }
...@@ -450,9 +448,20 @@ public class GossipLinkStore ...@@ -450,9 +448,20 @@ public class GossipLinkStore
450 return new DefaultLink(primaryProviderId , src, dst, type, annotations); 448 return new DefaultLink(primaryProviderId , src, dst, type, annotations);
451 } 449 }
452 450
453 - private ConcurrentMap<ProviderId, Timestamped<LinkDescription>> getLinkDescriptions(LinkKey key) { 451 + private Map<ProviderId, Timestamped<LinkDescription>> getLinkDescriptions(LinkKey key) {
454 - return ConcurrentUtils.createIfAbsentUnchecked(linkDescs, key, 452 + Map<ProviderId, Timestamped<LinkDescription>> r;
455 - NewConcurrentHashMap.<ProviderId, Timestamped<LinkDescription>>ifNeeded()); 453 + r = linkDescs.get(key);
454 + if (r != null) {
455 + return r;
456 + }
457 + r = new HashMap<>();
458 + final Map<ProviderId, Timestamped<LinkDescription>> concurrentlyAdded;
459 + concurrentlyAdded = linkDescs.putIfAbsent(key, r);
460 + if (concurrentlyAdded != null) {
461 + return concurrentlyAdded;
462 + } else {
463 + return r;
464 + }
456 } 465 }
457 466
458 private Timestamped<LinkDescription> getLinkDescription(LinkKey key, ProviderId providerId) { 467 private Timestamped<LinkDescription> getLinkDescription(LinkKey key, ProviderId providerId) {
...@@ -471,13 +480,13 @@ public class GossipLinkStore ...@@ -471,13 +480,13 @@ public class GossipLinkStore
471 } 480 }
472 } 481 }
473 482
474 - private static final Predicate<Provided> IS_PRIMARY = new IsPrimary();
475 - private static final Predicate<Provided> isPrimary() {
476 - return IS_PRIMARY;
477 - }
478 -
479 private static final class IsPrimary implements Predicate<Provided> { 483 private static final class IsPrimary implements Predicate<Provided> {
480 484
485 + private static final Predicate<Provided> IS_PRIMARY = new IsPrimary();
486 + public static final Predicate<Provided> isPrimary() {
487 + return IS_PRIMARY;
488 + }
489 +
481 @Override 490 @Override
482 public boolean apply(Provided input) { 491 public boolean apply(Provided input) {
483 return !input.providerId().isAncillary(); 492 return !input.providerId().isAncillary();
...@@ -582,11 +591,11 @@ public class GossipLinkStore ...@@ -582,11 +591,11 @@ public class GossipLinkStore
582 Map<LinkFragmentId, Timestamp> linkTimestamps = new HashMap<>(linkDescs.size()); 591 Map<LinkFragmentId, Timestamp> linkTimestamps = new HashMap<>(linkDescs.size());
583 Map<LinkKey, Timestamp> linkTombstones = new HashMap<>(removedLinks.size()); 592 Map<LinkKey, Timestamp> linkTombstones = new HashMap<>(removedLinks.size());
584 593
585 - for (Entry<LinkKey, ConcurrentMap<ProviderId, Timestamped<LinkDescription>>> 594 + for (Entry<LinkKey, Map<ProviderId, Timestamped<LinkDescription>>>
586 provs : linkDescs.entrySet()) { 595 provs : linkDescs.entrySet()) {
587 596
588 final LinkKey linkKey = provs.getKey(); 597 final LinkKey linkKey = provs.getKey();
589 - final ConcurrentMap<ProviderId, Timestamped<LinkDescription>> linkDesc = provs.getValue(); 598 + final Map<ProviderId, Timestamped<LinkDescription>> linkDesc = provs.getValue();
590 synchronized (linkDesc) { 599 synchronized (linkDesc) {
591 for (Map.Entry<ProviderId, Timestamped<LinkDescription>> e : linkDesc.entrySet()) { 600 for (Map.Entry<ProviderId, Timestamped<LinkDescription>> e : linkDesc.entrySet()) {
592 linkTimestamps.put(new LinkFragmentId(linkKey, e.getKey()), e.getValue().timestamp()); 601 linkTimestamps.put(new LinkFragmentId(linkKey, e.getKey()), e.getValue().timestamp());
......