Bug 564981 - Avoid reinserting service registration in lists if possible
If the service.ranking of a service is not changed by setProperties,
then the sort ordering of the service is unchanged and we can then
avoid removing and reinserting, at the correct sort index, the service
in the service lists.
Fixes https://bugs.eclipse.org/bugs/show_bug.cgi?id=564981
Change-Id: Ia94f187111073f8e629114812916c585b3fe8522
Signed-off-by: BJ Hargrave <hargrave@us.ibm.com>
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
index bdf4a0b..96d89dd 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
@@ -168,6 +168,7 @@
final ServiceReferenceImpl<S> ref;
final Map<String, Object> previousProperties;
synchronized (registry) {
+ int previousRanking;
synchronized (registrationLock) {
if (state != REGISTERED) { /* in the process of unregisterING */
throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION + ' ' + this);
@@ -175,9 +176,10 @@
ref = reference; /* used to publish event outside sync */
previousProperties = this.properties;
+ previousRanking = serviceranking;
this.properties = createProperties(props);
}
- registry.modifyServiceRegistration(context, this);
+ registry.modifyServiceRegistration(context, this, previousRanking);
}
/* must not hold the registrationLock when this event is published */
registry.publishServiceEvent(new ModifiedServiceEvent(ref, previousProperties));
@@ -746,23 +748,15 @@
*/
@Override
public int compareTo(ServiceRegistrationImpl<?> other) {
- final int thisRanking = this.getRanking();
- final int otherRanking = other.getRanking();
- if (thisRanking != otherRanking) {
- if (thisRanking < otherRanking) {
- return 1;
- }
- return -1;
+ return compareTo(other.getRanking(), other.getId());
+ }
+
+ int compareTo(int otherRanking, long otherId) {
+ int compared = Integer.compare(otherRanking, getRanking());
+ if (compared != 0) {
+ return compared;
}
- final long thisId = this.getId();
- final long otherId = other.getId();
- if (thisId == otherId) {
- return 0;
- }
- if (thisId < otherId) {
- return -1;
- }
- return 1;
+ return Long.compare(getId(), otherId);
}
static class FrameworkHookRegistration<S> extends ServiceRegistrationImpl<S> {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
index d379291..7d05058 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
@@ -1004,28 +1004,33 @@
* @param registration The modified ServiceRegistration.
*/
/* @GuardedBy("this") */
- void modifyServiceRegistration(BundleContextImpl context, ServiceRegistrationImpl<?> registration) {
+ void modifyServiceRegistration(BundleContextImpl context, ServiceRegistrationImpl<?> registration,
+ int previousRanking) {
assert Thread.holdsLock(this);
// The list of Services published by BundleContextImpl is not sorted, so
// we do not need to modify it.
- // Remove the ServiceRegistrationImpl from the list of Services published by Class Name
- // and then add at the correct index.
- int insertIndex;
- for (String clazz : registration.getClasses()) {
- List<ServiceRegistrationImpl<?>> services = publishedServicesByClass.get(clazz);
- services.remove(registration);
- // The list is sorted, so we must find the proper location to insert
- insertIndex = -Collections.binarySearch(services, registration) - 1;
- services.add(insertIndex, registration);
- }
+ // If the insert location has changed
+ if (registration.compareTo(previousRanking, registration.getId()) != 0) {
+ // Remove the ServiceRegistrationImpl from the list of Services published by
+ // Class Name
+ // and then add at the correct index.
+ int insertIndex;
+ for (String clazz : registration.getClasses()) {
+ List<ServiceRegistrationImpl<?>> services = publishedServicesByClass.get(clazz);
+ services.remove(registration);
+ // The list is sorted, so we must find the proper location to insert
+ insertIndex = -1 - Collections.binarySearch(services, registration);
+ services.add(insertIndex, registration);
+ }
- // Remove the ServiceRegistrationImpl from the list of all published Services
- // and then add at the correct index.
- allPublishedServices.remove(registration);
- // The list is sorted, so we must find the proper location to insert
- insertIndex = -Collections.binarySearch(allPublishedServices, registration) - 1;
- allPublishedServices.add(insertIndex, registration);
+ // Remove the ServiceRegistrationImpl from the list of all published Services
+ // and then add at the correct index.
+ allPublishedServices.remove(registration);
+ // The list is sorted, so we must find the proper location to insert
+ insertIndex = -1 - Collections.binarySearch(allPublishedServices, registration);
+ allPublishedServices.add(insertIndex, registration);
+ }
}
/**
