Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache results of isFiatCurrency and isCryptoCurrency #4955

Merged
merged 3 commits into from
Dec 16, 2020
Merged

Cache results of isFiatCurrency and isCryptoCurrency #4955

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5112b1a
Cache results of isFiatCurrency and isCryptoCurrency
chimp1984 Dec 15, 2020
0f084d3
Use ConcurrentHashMap
chimp1984 Dec 15, 2020
ecad724
Add null checks, improve comment
chimp1984 Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 56 additions & 23 deletions core/src/main/java/bisq/core/locale/CurrencyUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
Expand All @@ -53,7 +54,6 @@

@Slf4j
public class CurrencyUtil {

public static void setup() {
setBaseCurrencyCode(Config.baseCurrencyNetwork().getCurrencyCode());
}
Expand All @@ -62,6 +62,14 @@ public static void setup() {

private static String baseCurrencyCode = "BTC";

// Calls to isFiatCurrency and isCryptoCurrency are very frequent so we use a cache of the results.
// The main improvement was already achieved with using memoize for the source maps, but
// the caching still reduces performance costs by about 20% for isCryptoCurrency (1752 ms vs 2121 ms) and about 50%
// for isFiatCurrency calls (1777 ms vs 3467 ms).
// See: https://github.com/bisq-network/bisq/pull/4955#issuecomment-745302802
private static final Map<String, Boolean> isFiatCurrencyMap = new ConcurrentHashMap<>();
private static final Map<String, Boolean> isCryptoCurrencyMap = new ConcurrentHashMap<>();

private static Supplier<Map<String, FiatCurrency>> fiatCurrencyMapSupplier = Suppliers.memoize(
CurrencyUtil::createFiatCurrencyMap)::get;
private static Supplier<Map<String, CryptoCurrency>> cryptoCurrencyMapSupplier = Suppliers.memoize(
Expand Down Expand Up @@ -391,12 +399,25 @@ public static List<TradeCurrency> getMatureMarketCurrencies() {
}

public static boolean isFiatCurrency(String currencyCode) {
if (currencyCode != null && isFiatCurrencyMap.containsKey(currencyCode)) {
return isFiatCurrencyMap.get(currencyCode);
}

try {
return currencyCode != null
boolean isFiatCurrency = currencyCode != null
&& !currencyCode.isEmpty()
&& !isCryptoCurrency(currencyCode)
&& Currency.getInstance(currencyCode) != null;

if (currencyCode != null) {
isFiatCurrencyMap.put(currencyCode, isFiatCurrency);
}

return isFiatCurrency;
} catch (Throwable t) {
if (currencyCode != null) {
isFiatCurrencyMap.put(currencyCode, false);
}
return false;
}
}
Expand All @@ -417,28 +438,37 @@ public static Optional<FiatCurrency> getFiatCurrency(String currencyCode) {
* contains 3 entries (CryptoCurrency, Fiat, Undefined).
*/
public static boolean isCryptoCurrency(String currencyCode) {
// Some tests call that method with null values. Should be fixed in the tests but to not break them return false.
if (currencyCode == null)
return false;

// BTC is not part of our assetRegistry so treat it extra here. Other old base currencies (LTC, DOGE, DASH)
// are not supported anymore so we can ignore that case.
if (currencyCode.equals("BTC"))
return true;
if (currencyCode != null && isCryptoCurrencyMap.containsKey(currencyCode)) {
return isCryptoCurrencyMap.get(currencyCode);
}

// If we find the code in our assetRegistry we return true.
// It might be that an asset was removed from the assetsRegistry, we deal with such cases below by checking if
// it is a fiat currency
if (getCryptoCurrency(currencyCode).isPresent())
return true;
boolean isCryptoCurrency;
if (currencyCode == null) {
// Some tests call that method with null values. Should be fixed in the tests but to not break them return false.
isCryptoCurrency = false;
} else if (currencyCode.equals("BTC")) {
// BTC is not part of our assetRegistry so treat it extra here. Other old base currencies (LTC, DOGE, DASH)
// are not supported anymore so we can ignore that case.
isCryptoCurrency = true;
} else if (getCryptoCurrency(currencyCode).isPresent()) {
// If we find the code in our assetRegistry we return true.
// It might be that an asset was removed from the assetsRegistry, we deal with such cases below by checking if
// it is a fiat currency
isCryptoCurrency = true;
} else if (!getFiatCurrency(currencyCode).isPresent()) {
// In case the code is from a removed asset we cross check if there exist a fiat currency with that code,
// if we don't find a fiat currency we treat it as a crypto currency.
isCryptoCurrency = true;
} else {
// If we would have found a fiat currency we return false
isCryptoCurrency = false;
}

// In case the code is from a removed asset we cross check if there exist a fiat currency with that code,
// if we don't find a fiat currency we treat it as a crypto currency.
if (!getFiatCurrency(currencyCode).isPresent())
return true;
if (currencyCode != null) {
isCryptoCurrencyMap.put(currencyCode, isCryptoCurrency);
}

// If we would have found a fiat currency we return false
return false;
return isCryptoCurrency;
}

public static Optional<CryptoCurrency> getCryptoCurrency(String currencyCode) {
Expand Down Expand Up @@ -527,7 +557,9 @@ private static CryptoCurrency assetToCryptoCurrency(Asset asset) {
return new CryptoCurrency(asset.getTickerSymbol(), asset.getName(), asset instanceof Token);
}

private static boolean isNotBsqOrBsqTradingActivated(Asset asset, BaseCurrencyNetwork baseCurrencyNetwork, boolean daoTradingActivated) {
private static boolean isNotBsqOrBsqTradingActivated(Asset asset,
BaseCurrencyNetwork baseCurrencyNetwork,
boolean daoTradingActivated) {
return !(asset instanceof BSQ) ||
daoTradingActivated && assetMatchesNetwork(asset, baseCurrencyNetwork);
}
Expand Down Expand Up @@ -582,7 +614,8 @@ public static Optional<Asset> findAsset(String tickerSymbol, BaseCurrencyNetwork
}

// Excludes all assets which got removed by DAO voting
public static List<CryptoCurrency> getActiveSortedCryptoCurrencies(AssetService assetService, FilterManager filterManager) {
public static List<CryptoCurrency> getActiveSortedCryptoCurrencies(AssetService assetService,
FilterManager filterManager) {
return getAllSortedCryptoCurrencies().stream()
.filter(e -> e.getCode().equals("BSQ") || assetService.isActive(e.getCode()))
.filter(e -> !filterManager.isCurrencyBanned(e.getCode()))
Expand Down