From: Robert Varga Date: Wed, 30 Jul 2014 18:42:00 +0000 (+0200) Subject: BUG-1228: Optimize AbstractForwardedDataBroker a bit X-Git-Tag: release/helium~391 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=2aa99b3ddb662c25490fbcbf9f65d99a6811e29a BUG-1228: Optimize AbstractForwardedDataBroker a bit Profiling OFP with datastore backend has revealed that we spend about 9% of CPU in AbstractForwardedDataBroker$1.compare(). This turns out to be a comparator, which compares two YangInstanceIdentifiers by their path length -- and to do that it iterates fully through both of them. We perform three optimizations: - make the comparator itself shareable, so we do not instantiate it (6000 during the profiling run) - make the .sortedEntries() smart about empty maps - teach the comparator to abort iteration as soon as the result is known At the same time we import YangInstanceIdentifier, making the code a bit more readable. Change-Id: I38d3629ab33dac9e2e29b092e3a89d0821d09b07 Signed-off-by: Robert Varga --- diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java index 838a02ba79..15e4a466cf 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java @@ -7,11 +7,15 @@ */ package org.opendaylight.controller.md.sal.binding.impl; +import com.google.common.base.Objects; +import com.google.common.base.Optional; + import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -31,6 +35,7 @@ import org.opendaylight.yangtools.concepts.Delegator; import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.impl.codec.BindingIndependentMappingService; import org.opendaylight.yangtools.yang.data.impl.codec.DeserializationException; @@ -40,12 +45,7 @@ import org.opendaylight.yangtools.yang.model.api.SchemaServiceListener; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Objects; -import com.google.common.base.Optional; -import com.google.common.collect.Iterables; - -public abstract class AbstractForwardedDataBroker implements Delegator, DomForwardedBroker, - SchemaContextListener, AutoCloseable { +public abstract class AbstractForwardedDataBroker implements Delegator, DomForwardedBroker, SchemaContextListener, AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(AbstractForwardedDataBroker.class); // The Broker to whom we do all forwarding @@ -91,17 +91,17 @@ public abstract class AbstractForwardedDataBroker implements Delegator domRegistration = domDataBroker.registerDataChangeListener(store, domPath, domDataChangeListener, triggeringScope); return new ListenerRegistrationImpl(listener, domRegistration); } protected Map, DataObject> toBinding( - final Map> normalized) { + final Map> normalized) { Map, DataObject> newMap = new HashMap<>(); - for (Map.Entry> entry : sortedEntries(normalized)) { + for (Map.Entry> entry : sortedEntries(normalized)) { try { Optional, DataObject>> potential = getCodec().toBinding( entry); @@ -116,25 +116,43 @@ public abstract class AbstractForwardedDataBroker implements Delegator Iterable> sortedEntries(final Map map) { - ArrayList> entries = new ArrayList<>(map.entrySet()); - Collections.sort(entries, new Comparator>() { + private static final Comparator> MAP_ENTRY_COMPARATOR = new Comparator>() { + @Override + public int compare(final Entry left, + final Entry right) { + final Iterator li = left.getKey().getPathArguments().iterator(); + final Iterator ri = right.getKey().getPathArguments().iterator(); + + // Iterate until left is exhausted... + while (li.hasNext()) { + if (!ri.hasNext()) { + // Left is deeper + return 1; + } - @Override - public int compare(final Entry left, - final Entry right) { - int leftSize = Iterables.size(left.getKey().getPathArguments()); - int rightSize = Iterables.size(right.getKey().getPathArguments()); - return Integer.compare(leftSize, rightSize); + li.next(); + ri.next(); } - }); - return entries; + + // Check if right is exhausted + return ri.hasNext() ? -1 : 0; + } + }; + + private static Iterable> sortedEntries(final Map map) { + if (!map.isEmpty()) { + ArrayList> entries = new ArrayList<>(map.entrySet()); + Collections.sort(entries, MAP_ENTRY_COMPARATOR); + return entries; + } else { + return Collections.emptySet(); + } } protected Set> toBinding( - final Set normalized) { + final Set normalized) { Set> hashSet = new HashSet<>(); - for (org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier normalizedPath : normalized) { + for (YangInstanceIdentifier normalizedPath : normalized) { try { Optional> potential = getCodec().toBinding(normalizedPath); if (potential.isPresent()) { @@ -176,13 +194,13 @@ public abstract class AbstractForwardedDataBroker implements Delegator> change) { + final AsyncDataChangeEvent> change) { bindingDataChangeListener.onDataChanged(new TranslatedDataChangeEvent(change, path)); } } private class TranslatedDataChangeEvent implements AsyncDataChangeEvent, DataObject> { - private final AsyncDataChangeEvent> domEvent; + private final AsyncDataChangeEvent> domEvent; private final InstanceIdentifier path; private Map, DataObject> createdCache; @@ -193,7 +211,7 @@ public abstract class AbstractForwardedDataBroker implements Delegator updatedDataCache; public TranslatedDataChangeEvent( - final AsyncDataChangeEvent> change, + final AsyncDataChangeEvent> change, final InstanceIdentifier path) { this.domEvent = change; this.path = path;