diff --git a/xchange-core/src/main/java/org/knowm/xchange/dto/marketdata/OrderBook.java b/xchange-core/src/main/java/org/knowm/xchange/dto/marketdata/OrderBook.java index ec8eb77b271..e7c8adabd3f 100644 --- a/xchange-core/src/main/java/org/knowm/xchange/dto/marketdata/OrderBook.java +++ b/xchange-core/src/main/java/org/knowm/xchange/dto/marketdata/OrderBook.java @@ -142,19 +142,24 @@ private void update(List limitOrders, LimitOrder limitOrder) { long writeStamp = lock.tryConvertToWriteLock(stamp); if (writeStamp != 0L) { stamp = writeStamp; - if (idx >= 0) limitOrders.remove(idx); - else idx = -idx - 1; - if (limitOrder.getRemainingAmount().compareTo(BigDecimal.ZERO) != 0) + if (idx >= 0) { + limitOrders.remove(idx); + } else { + idx = -idx - 1; + } + if (limitOrder.getRemainingAmount().compareTo(BigDecimal.ZERO) != 0) { limitOrders.add(idx, limitOrder); + } updateDate(limitOrder.getTimestamp()); break; } else { lock.unlockRead(stamp); stamp = lock.writeLock(); // here wee need to recheck idx, because it is possible that orderBook changed between - // unlockRead and lockWrite - if (recheckIdx(limitOrders, limitOrder, idx)) + // unlockRead and writeLock + if (recheckIdx(limitOrders, limitOrder, idx)) { idx = Collections.binarySearch(limitOrders, limitOrder); + } } } } finally { @@ -179,8 +184,11 @@ public void update(OrderBookUpdate orderBookUpdate) { long writeStamp = lock.tryConvertToWriteLock(stamp); if (writeStamp != 0L) { stamp = writeStamp; - if (idx >= 0) limitOrders.remove(idx); - else idx = -idx - 1; + if (idx >= 0) { + limitOrders.remove(idx); + } else { + idx = -idx - 1; + } if (orderBookUpdate.getTotalVolume().compareTo(BigDecimal.ZERO) != 0) { LimitOrder updatedOrder = withAmount(limitOrder, orderBookUpdate.getTotalVolume()); limitOrders.add(idx, updatedOrder); @@ -192,8 +200,9 @@ public void update(OrderBookUpdate orderBookUpdate) { stamp = lock.writeLock(); // here wee need to recheck idx, because it is possible that orderBook changed between // unlockRead and lockWrite - if (recheckIdx(limitOrders, limitOrder, idx)) + if (recheckIdx(limitOrders, limitOrder, idx)) { idx = Collections.binarySearch(limitOrders, limitOrder); + } } } } finally { @@ -201,22 +210,26 @@ public void update(OrderBookUpdate orderBookUpdate) { } } + /** + * @return true, if wee need to run binarySearch again + */ private boolean recheckIdx(List limitOrders, LimitOrder limitOrder, int idx) { - if (idx >= 0) { - // if positive, null check or compare - return limitOrders.get(idx) == null || limitOrders.get(idx).compareTo(limitOrder) != 0; - } else { - // on end of array, null check or one check - if (limitOrders.size() == -idx - 1) { - return limitOrders.get(-idx - 2) == null - || limitOrders.get(-idx - 2).compareTo(limitOrder) >= 0; - } else - // if negative, check that of limitOrders.get(reversed idx) limitOrders.get(reversed idx-1) - // and is lower and bigger than limitOrder - return (limitOrders.get(-idx - 1) == null - || limitOrders.get(-idx - 1).compareTo(limitOrder) <= 0) - && (limitOrders.get(-idx - 2) == null - || limitOrders.get(-idx - 2).compareTo(limitOrder) >= 0); + switch (idx) { + case 0: + { + if (!limitOrders.isEmpty()) { + // if not equals, need to recheck + return limitOrders.get(0).compareTo(limitOrder) != 0; + } else return true; + } + case -1: + { + if (limitOrders.isEmpty()) { + return false; + } else return limitOrders.get(0).compareTo(limitOrder) <= 0; + } + default: + return true; } } diff --git a/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/ConcurrencyTest.java b/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/ConcurrencyTest.java index a00d8abcabe..3473d2f3208 100644 --- a/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/ConcurrencyTest.java +++ b/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/ConcurrencyTest.java @@ -19,12 +19,10 @@ public class ConcurrencyTest { static Instrument inst = new CurrencyPair("BTC/USDT"); public static void main(String[] args) throws InterruptedException, ExecutionException { - OrderBook orderBook1 = - new OrderBook(new Date(), initOrderBookAsks(), initOrderBookBids(), true); - OrderBook orderBook2 = - new OrderBook(new Date(), initOrderBookAsks(), initOrderBookBids(), true); + OrderBook orderBook1 = new OrderBook(new Date(), initOrderBookAsks(), new ArrayList<>(), true); + OrderBook orderBook2 = new OrderBook(new Date(), initOrderBookAsks(), new ArrayList<>(), true); OrderBookOld orderBookOld = - new OrderBookOld(new Date(), initOrderBookAsks(), initOrderBookBids(), true); + new OrderBookOld(new Date(), initOrderBookAsks(), new ArrayList<>(), true); ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(50); newWay(orderBook1, executor); executor.awaitTermination(100L, TimeUnit.SECONDS); @@ -114,7 +112,7 @@ private static void oldOB(OrderBookOld orderBookOld, ThreadPoolExecutor executor public static void updateOrderBook1(OrderBook orderBook, boolean oldWay) { Random rand = new Random(123); - for (int i = 0; i < 100000; i++) { + for (int i = 0; i < 10000; i++) { OrderBookUpdate orderBookUpdateAsk = new OrderBookUpdate( OrderType.ASK, @@ -145,7 +143,7 @@ public static void updateOrderBook1(OrderBook orderBook, boolean oldWay) { public static void updateOrderBookOld1(OrderBookOld orderBookOld) { Random rand = new Random(123); - for (int i = 0; i < 100000; i++) { + for (int i = 0; i < 10000; i++) { OrderBookUpdate orderBookUpdateAsk = new OrderBookUpdate( OrderType.ASK, @@ -171,7 +169,7 @@ public static void updateOrderBookOld1(OrderBookOld orderBookOld) { private static void updateOrderBook2(OrderBook orderBook, boolean oldWay) { Random rand = new Random(123); - for (int i = 0; i < 100000; i++) { + for (int i = 0; i < 10000; i++) { LimitOrder bookUpdateAsk = new LimitOrder( OrderType.ASK, @@ -202,7 +200,7 @@ private static void updateOrderBook2(OrderBook orderBook, boolean oldWay) { private static void updateOrderBookOld2(OrderBookOld orderBookOld) { Random rand = new Random(123); - for (int i = 0; i < 100000; i++) { + for (int i = 0; i < 10000; i++) { LimitOrder bookUpdateAsk = new LimitOrder( OrderType.ASK, @@ -227,7 +225,7 @@ private static void updateOrderBookOld2(OrderBookOld orderBookOld) { } private static void readOrderBook(OrderBook orderBook, boolean oldWay) { - for (int i = 0; i < 1200000; i++) { + for (int i = 0; i < 10000; i++) { int temp = 0; if (oldWay) { synchronized (orderBook) { @@ -252,7 +250,7 @@ private static void readOrderBook(OrderBook orderBook, boolean oldWay) { } private static void readOrderBookOld(OrderBookOld orderBookOld) { - for (int i = 0; i < 1200000; i++) { + for (int i = 0; i < 120000; i++) { int temp = 0; synchronized (orderBookOld) { for (LimitOrder ask : orderBookOld.getAsks()) { diff --git a/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/OrderBookTest.java b/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/OrderBookTest.java index dffe01b43e2..20906f72d48 100644 --- a/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/OrderBookTest.java +++ b/xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/OrderBookTest.java @@ -3,6 +3,8 @@ import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.math.BigDecimal; import java.time.Duration; import java.util.ArrayList; @@ -174,4 +176,43 @@ public void testOrderSorting() { assertThat(book.getAsks().get(0).getLimitPrice()).isEqualTo(new BigDecimal("10")); assertThat(book.getBids().get(0).getLimitPrice()).isEqualTo(new BigDecimal("3")); } + + @Test + public void testRecheckIdx() + throws IOException, NoSuchMethodException, InvocationTargetException, IllegalAccessException { + Class[] cArg = new Class[3]; + cArg[0] = List.class; + cArg[1] = LimitOrder.class; + cArg[2] = int.class; + Method method = OrderBook.class.getDeclaredMethod("recheckIdx", cArg); + method.setAccessible(true); + LimitOrder sameBidOrder = + new LimitOrder( + OrderType.BID, BigDecimal.ONE, CurrencyPair.BTC_USD, "", null, BigDecimal.TEN); + LimitOrder smallerBidOrder = + new LimitOrder( + OrderType.BID, BigDecimal.ONE, CurrencyPair.BTC_USD, "", null, BigDecimal.ONE); + LimitOrder higherBidOrder = + new LimitOrder( + OrderType.BID, BigDecimal.ONE, CurrencyPair.BTC_USD, "", null, new BigDecimal("10.5")); + // idx!= -1,0 + assertThat(method.invoke(orderBook, new ArrayList(), sameBidOrder, 1)) + .isEqualTo(true); + // idx=0, empty List + assertThat(method.invoke(orderBook, new ArrayList(), sameBidOrder, 0)) + .isEqualTo(true); + // idx=0, order equals + assertThat(method.invoke(orderBook, orderBook.getBids(), sameBidOrder, 0)).isEqualTo(false); + // idx=0, smallerBidOrder + assertThat(method.invoke(orderBook, orderBook.getBids(), smallerBidOrder, 0)).isEqualTo(true); + // idx=-1, empty List + assertThat(method.invoke(orderBook, new ArrayList(), sameBidOrder, -1)) + .isEqualTo(false); + // idx=-1, order equals + assertThat(method.invoke(orderBook, orderBook.getBids(), sameBidOrder, -1)).isEqualTo(true); + // idx=-1, smaller order + assertThat(method.invoke(orderBook, orderBook.getBids(), smallerBidOrder, -1)).isEqualTo(true); + // idx=-1, higher order + assertThat(method.invoke(orderBook, orderBook.getBids(), higherBidOrder, -1)).isEqualTo(false); + } }