Skip to content

Commit

Permalink
Improve catalog perf for Mutable catalog apis by remove Iterables met…
Browse files Browse the repository at this point in the history 
…hod (not very suited for arrays). See #621
  • Loading branch information
@sbrossie
sbrossie committed Sep 29, 2016
1 parent abdd73d commit af82d2d
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 67 deletions.
36 changes: 15 additions & 21 deletions catalog/src/main/java/org/killbill/billing/catalog/CatalogUpdater.java
View file
Expand Up @@ -44,10 +44,6 @@
import org.killbill.billing.catalog.rules.DefaultPlanRules; import org.killbill.billing.catalog.rules.DefaultPlanRules;
import org.killbill.xmlloader.XMLWriter; import org.killbill.xmlloader.XMLWriter;


import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;

public class CatalogUpdater { public class CatalogUpdater {


private static URI DUMMY_URI; private static URI DUMMY_URI;
Expand Down Expand Up @@ -253,15 +249,13 @@ private void validateExistingPlan(final DefaultPlan plan, final SimplePlanDescri


private boolean isCurrencySupported(final Currency targetCurrency) { private boolean isCurrencySupported(final Currency targetCurrency) {
if (catalog.getCurrentSupportedCurrencies() != null) { if (catalog.getCurrentSupportedCurrencies() != null) {
return Iterables.any(ImmutableList.copyOf(catalog.getCurrentSupportedCurrencies()), new Predicate<Currency>() { for (final Currency input : catalog.getCurrentSupportedCurrencies()) {
@Override if (input.equals(targetCurrency)) {
public boolean apply(final Currency input) { return true;
return input.equals(targetCurrency);
} }
}); }
} else {
return false;
} }
return false;
} }


private void validateNewPlanDescriptor(final SimplePlanDescriptor desc) throws CatalogApiException { private void validateNewPlanDescriptor(final SimplePlanDescriptor desc) throws CatalogApiException {
Expand All @@ -285,21 +279,21 @@ private void validateNewPlanDescriptor(final SimplePlanDescriptor desc) throws C
} }


private DefaultProduct getExistingProduct(final String productName) { private DefaultProduct getExistingProduct(final String productName) {
return Iterables.tryFind(ImmutableList.copyOf(catalog.getCurrentProducts()), new Predicate<DefaultProduct>() { for (final DefaultProduct input : catalog.getCurrentProducts()) {
@Override if (input.getName().equals(productName)) {
public boolean apply(final DefaultProduct input) { return input;
return input.getName().equals(productName);
} }
}).orNull(); }
return null;
} }


private DefaultPlan getExistingPlan(final String planName) { private DefaultPlan getExistingPlan(final String planName) {
return Iterables.tryFind(ImmutableList.copyOf(catalog.getCurrentPlans()), new Predicate<DefaultPlan>() { for (final DefaultPlan input : catalog.getCurrentPlans()) {
@Override if (input.getName().equals(planName)) {
public boolean apply(final DefaultPlan input) { return input;
return input.getName().equals(planName);
} }
}).orNull(); }
return null;
} }


private DefaultPlanRules getSaneDefaultPlanRules(final DefaultPriceList defaultPriceList) { private DefaultPlanRules getSaneDefaultPlanRules(final DefaultPriceList defaultPriceList) {
Expand Down
80 changes: 34 additions & 46 deletions catalog/src/main/java/org/killbill/billing/catalog/DefaultMutableStaticCatalog.java
View file
Expand Up @@ -18,27 +18,19 @@
package org.killbill.billing.catalog; package org.killbill.billing.catalog;


import java.lang.reflect.Array; import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Date; import java.util.Date;
import java.util.List; import java.util.List;


import javax.annotation.Nullable;

import org.killbill.billing.catalog.api.CatalogApiException; import org.killbill.billing.catalog.api.CatalogApiException;
import org.killbill.billing.catalog.api.CatalogEntity; import org.killbill.billing.catalog.api.CatalogEntity;
import org.killbill.billing.catalog.api.Currency; import org.killbill.billing.catalog.api.Currency;
import org.killbill.billing.catalog.api.InternationalPrice;
import org.killbill.billing.catalog.api.MutableStaticCatalog; import org.killbill.billing.catalog.api.MutableStaticCatalog;
import org.killbill.billing.catalog.api.Plan; import org.killbill.billing.catalog.api.Plan;
import org.killbill.billing.catalog.api.PlanPhasePriceOverridesWithCallContext;
import org.killbill.billing.catalog.api.PlanSpecifier;
import org.killbill.billing.catalog.api.Price; import org.killbill.billing.catalog.api.Price;
import org.killbill.billing.catalog.api.PriceList; import org.killbill.billing.catalog.api.PriceList;
import org.killbill.billing.catalog.api.Product; import org.killbill.billing.catalog.api.Product;


import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;

public class DefaultMutableStaticCatalog extends StandaloneCatalog implements MutableStaticCatalog { public class DefaultMutableStaticCatalog extends StandaloneCatalog implements MutableStaticCatalog {


public DefaultMutableStaticCatalog() { public DefaultMutableStaticCatalog() {
Expand All @@ -61,10 +53,9 @@ public DefaultMutableStaticCatalog(final StandaloneCatalog input) {
initialize(this, null); initialize(this, null);
} }



@Override @Override
public void addCurrency(final Currency currency) throws CatalogApiException { public void addCurrency(final Currency currency) throws CatalogApiException {
final Currency [] newEntries = allocateNewEntries(getCurrentSupportedCurrencies(), currency); final Currency[] newEntries = allocateNewEntries(getCurrentSupportedCurrencies(), currency);
setSupportedCurrencies(newEntries); setSupportedCurrencies(newEntries);
} }


Expand All @@ -79,28 +70,25 @@ public void addPlan(final Plan plan) throws CatalogApiException {
final Plan[] newEntries = allocateNewEntries(getCurrentPlans(), plan); final Plan[] newEntries = allocateNewEntries(getCurrentPlans(), plan);
setPlans((DefaultPlan[]) newEntries); setPlans((DefaultPlan[]) newEntries);



final DefaultPriceList priceList = getPriceLists().findPriceListFrom(plan.getPriceListName()); final DefaultPriceList priceList = getPriceLists().findPriceListFrom(plan.getPriceListName());

final List<DefaultPlan> newPriceListPlan = new ArrayList<DefaultPlan>();
final Iterable<DefaultPlan> newPriceListPlan = Iterables.filter(ImmutableList.copyOf((DefaultPlan[]) newEntries), new Predicate<DefaultPlan>() { for (Plan input : newEntries) {
@Override if (plan.getName().equals(input.getName())) {
public boolean apply(final DefaultPlan input) { newPriceListPlan.add((DefaultPlan) plan);
if (plan.getName().equals(input.getName())) { continue;
return true; }
} if (priceList.getPlans() != null) {
if (priceList.getPlans() != null) { for (final Plan priceListPlan : priceList.getPlans()) {
for (final Plan priceListPlan : priceList.getPlans()) { if (priceListPlan.getName().equals(input.getName())) {
if (priceListPlan.getName().equals(input.getName())) { newPriceListPlan.add((DefaultPlan) priceListPlan);
return true; break;
}
} }
} }
return false;
} }
}); }
final List<DefaultPlan> foo = ImmutableList.<DefaultPlan >copyOf(newPriceListPlan);
final Plan[] newPriceListEntries = new DefaultPlan[foo.size()]; final Plan[] newPriceListEntries = new DefaultPlan[newPriceListPlan.size()];
final Plan[] bar = foo.toArray(newPriceListEntries); final Plan[] bar = newPriceListPlan.toArray(newPriceListEntries);
priceList.setPlans((DefaultPlan[]) bar); priceList.setPlans((DefaultPlan[]) bar);
} }


Expand All @@ -111,41 +99,41 @@ public void addPriceList(final PriceList priceList) throws CatalogApiException {
setPriceLists(priceListSet); setPriceLists(priceListSet);
} }



public void addRecurringPriceToPlan(final DefaultInternationalPrice currentPrices, final Price newPrice) throws CatalogApiException { public void addRecurringPriceToPlan(final DefaultInternationalPrice currentPrices, final Price newPrice) throws CatalogApiException {
final Price [] newEntries = allocateNewEntries(currentPrices.getPrices(), newPrice); final Price[] newEntries = allocateNewEntries(currentPrices.getPrices(), newPrice);
currentPrices.setPrices((DefaultPrice []) newEntries); currentPrices.setPrices((DefaultPrice[]) newEntries);
} }


public void addProductAvailableAO(final DefaultProduct targetBasePlan, final DefaultProduct aoProduct) throws CatalogApiException { public void addProductAvailableAO(final DefaultProduct targetBasePlan, final DefaultProduct aoProduct) throws CatalogApiException {
final Product[] newEntries = allocateNewEntries(targetBasePlan.getAvailable(), aoProduct); final Product[] newEntries = allocateNewEntries(targetBasePlan.getAvailable(), aoProduct);
targetBasePlan.setAvailable((DefaultProduct[]) newEntries); targetBasePlan.setAvailable((DefaultProduct[]) newEntries);
} }


private <T> T [] allocateNewEntries(final T [] existingEntries, final T newEntry) throws CatalogApiException { private <T> T[] allocateNewEntries(final T[] existingEntries, final T newEntry) throws CatalogApiException {


// Verify entry does not already exists if (existingEntries != null) {
if (existingEntries != null && Iterables.any(ImmutableList.<T>copyOf(existingEntries), new Predicate<T>() { for (T input : existingEntries) {
@Override boolean found;
public boolean apply(final T input) {
if (input instanceof CatalogEntity) { if (input instanceof CatalogEntity) {
return ((CatalogEntity) input).getName().equals(((CatalogEntity) newEntry).getName()); found = ((CatalogEntity) input).getName().equals(((CatalogEntity) newEntry).getName());
} else if (input instanceof Enum) { } else if (input instanceof Enum) {
return ((Enum) input).name().equals(((Enum) newEntry).name()); found = ((Enum) input).name().equals(((Enum) newEntry).name());
} else if (input instanceof Price) { } else if (input instanceof Price) {
return ((Price) input).getCurrency().equals(((Price) newEntry).getCurrency()); found = ((Price) input).getCurrency().equals(((Price) newEntry).getCurrency());
} else {
throw new IllegalStateException("Unexpected type " + newEntry.getClass());
}
if (found) {
//throw new CatalogApiException();
throw new IllegalStateException("Already existing " + newEntry);
} }
throw new IllegalStateException("Unexpected type " + newEntry.getClass());
} }
})) {
//throw new CatalogApiException();
throw new IllegalStateException("Already existing " + newEntry);
} }


// Realloc and assign new entry // Realloc and assign new entry
final int length = existingEntries != null ? existingEntries.length : 0; final int length = existingEntries != null ? existingEntries.length : 0;
final T [] newEntries = (T[]) Array.newInstance(newEntry.getClass(), length + 1); final T[] newEntries = (T[]) Array.newInstance(newEntry.getClass(), length + 1);
for (int i = 0 ; i < newEntries.length + 1; i++) { for (int i = 0; i < newEntries.length + 1; i++) {
if (i < newEntries.length - 1) { if (i < newEntries.length - 1) {
newEntries[i] = existingEntries[i]; newEntries[i] = existingEntries[i];
} else { } else {
Expand Down

1 comment on commit af82d2d

@pierre
Copy link
Member

@pierre pierre commented on af82d2d Oct 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Please sign in to comment.