Skip to content

Commit

Permalink
Merge pull request #4969 from rizer1980/PR/orderbook-recheckidx-debug
Browse files Browse the repository at this point in the history 
[CORE] orderbook recheckidx fix
  • Loading branch information
@timmolter
timmolter authored Jan 7, 2025
2 parents 682bd28 + a1bd6c0 commit 0b28513
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 34 deletions.
59 changes: 36 additions & 23 deletions xchange-core/src/main/java/org/knowm/xchange/dto/marketdata/OrderBook.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,24 @@ private void update(List<LimitOrder> 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 {
Expand All @@ -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);
Expand All @@ -192,31 +200,36 @@ 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 {
lock.unlock(stamp);
}
}

/**
* @return true, if wee need to run binarySearch again
*/
private boolean recheckIdx(List<LimitOrder> 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;
}
}

Expand Down
20 changes: 9 additions & 11 deletions xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/ConcurrencyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand All @@ -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()) {
Expand Down
41 changes: 41 additions & 0 deletions xchange-core/src/test/java/org/knowm/xchange/dto/marketdata/OrderBookTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<LimitOrder>(), sameBidOrder, 1))
.isEqualTo(true);
// idx=0, empty List
assertThat(method.invoke(orderBook, new ArrayList<LimitOrder>(), 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<LimitOrder>(), 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);
}
}

0 comments on commit 0b28513

Please sign in to comment.