From cf54e5b5e165f964298a361f3f8c10e945c86769 Mon Sep 17 00:00:00 2001 From: XP Date: Thu, 15 Jan 2026 16:23:10 -0800 Subject: [PATCH 1/2] Misc perf improvements --- .../gg/xp/reevent/events/BasicEventQueue.java | 112 +++++++++++------- .../gg/xp/reevent/events/EventMaster.java | 1 + .../events/triggers/duties/Odin.java | 9 +- .../events/triggers/duties/UCoB.java | 2 +- .../sys/EnhancedReadWriteReentrantLock.java | 17 ++- .../gg/xp/xivsupport/sys/LockAdapter.java | 5 +- 6 files changed, 96 insertions(+), 50 deletions(-) diff --git a/reevent/src/main/java/gg/xp/reevent/events/BasicEventQueue.java b/reevent/src/main/java/gg/xp/reevent/events/BasicEventQueue.java index d7cadd201536..49673ac700b2 100644 --- a/reevent/src/main/java/gg/xp/reevent/events/BasicEventQueue.java +++ b/reevent/src/main/java/gg/xp/reevent/events/BasicEventQueue.java @@ -12,14 +12,16 @@ import java.util.Iterator; import java.util.List; import java.util.Queue; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.ThreadFactory; public class BasicEventQueue implements EventQueue { private static final Logger log = LoggerFactory.getLogger(BasicEventQueue.class); - private final Deque backingQueue = new ArrayDeque<>(); - private final Object queueLock = new Object(); + private final BlockingQueue backingQueue = new ArrayBlockingQueue<>(65536); +// private final Object queueLock = new Object(); private final List delayedEvents = new ArrayList<>(); private volatile boolean delayedEventsDirtyFlag; private static final ThreadFactory delayedEventProcessorThreadFactory = new BasicThreadFactory.Builder() @@ -59,25 +61,37 @@ public void push(Event event) { if (runAt == 0 || runAt <= System.currentTimeMillis()) { event.setEnqueuedAt(TimeUtils.now()); Tracker tracker = new Tracker(event); - synchronized (queueLock) { - if (enableCombine) { - Tracker existingTracker = backingQueue.peekLast(); - Event combined; - if (existingTracker == null || (combined = existingTracker.event.combineWith(event)) == null) { - backingQueue.add(existingTracker); - } - else { -// log.info("Combined!"); - backingQueue.removeLast(); - backingQueue.add(new Tracker(combined)); - } - } - else { - backingQueue.add(tracker); - } -// log.info("Push: {}", event); - queueLock.notifyAll(); + try { + backingQueue.put(tracker); } + catch (InterruptedException e) { + throw new RuntimeException(e); + } + // old impl +// synchronized (queueLock) { +// if (enableCombine) { +// Tracker existingTracker = backingQueue.peek(); +// Event combined; +// if (existingTracker == null || (combined = existingTracker.event.combineWith(event)) == null) { +// backingQueue.add(existingTracker); +// } +// else { +//// log.info("Combined!"); +// try { +// backingQueue.take(); +// } +// catch (InterruptedException e) { +// throw new RuntimeException(e); +// } +// backingQueue.add(new Tracker(combined)); +// } +// } +// else { +// backingQueue.add(tracker); +// } +//// log.info("Push: {}", event); +// queueLock.notifyAll(); +// } } else { queueDelayedEvent(event); @@ -86,32 +100,43 @@ public void push(Event event) { @Override public Event pull() { - synchronized (queueLock) { - while (true) { - Tracker tracker = backingQueue.poll(); - if (tracker == null) { - try { - queueLock.wait(5000); - } - catch (InterruptedException e) { - // ignored - } - } - else { - tracker.markExit(); - queueLock.notifyAll(); -// log.info("Pull: {}", tracker.event); - return tracker.event; - } - } + Tracker tracker; + try { + tracker = backingQueue.take(); + tracker.markExit(); + return tracker.event; } + catch (InterruptedException e) { + throw new RuntimeException(e); + } + // TODO: see if a lockless queue + wait only if nothing there might be better for performance. + // Old impl +// synchronized (queueLock) { +// while (true) { +// Tracker tracker = backingQueue.poll(); +// if (tracker == null) { +// try { +// queueLock.wait(5000); +// } +// catch (InterruptedException e) { +// // ignored +// } +// } +// else { +// tracker.markExit(); +// queueLock.notifyAll(); +//// log.info("Pull: {}", tracker.event); +// return tracker.event; +// } +// } +// } } @Override public int pendingSize() { - synchronized (queueLock) { +// synchronized (queueLock) { return backingQueue.size(); - } +// } } // Should only be used for testing, or maybe hot reloads @@ -119,16 +144,17 @@ public int pendingSize() { // event to be fully processed. This probably needs to be on EventMaster. @Override public void waitDrain() { - synchronized (queueLock) { +// synchronized (queueLock) { while (pendingSize() > 0) { try { - queueLock.wait(1000); + Thread.sleep(10); +// queueLock.wait(1000); } catch (InterruptedException e) { throw new RuntimeException(e); } } - } +// } } private void queueDelayedEvent(Event event) { diff --git a/reevent/src/main/java/gg/xp/reevent/events/EventMaster.java b/reevent/src/main/java/gg/xp/reevent/events/EventMaster.java index 32ad66cbde9a..c8ee49c0d31e 100644 --- a/reevent/src/main/java/gg/xp/reevent/events/EventMaster.java +++ b/reevent/src/main/java/gg/xp/reevent/events/EventMaster.java @@ -100,6 +100,7 @@ public void pushEvent(Event event) { queue.push(event); } + // TODO: this is not thread safe public void pushEventAndWait(Event event) { CountDownLatch latch = new CountDownLatch(1); drainCallback = latch::countDown; diff --git a/triggers/triggers-general/src/main/java/gg/xp/xivsupport/events/triggers/duties/Odin.java b/triggers/triggers-general/src/main/java/gg/xp/xivsupport/events/triggers/duties/Odin.java index 8650963da024..5df15b4ac2cb 100644 --- a/triggers/triggers-general/src/main/java/gg/xp/xivsupport/events/triggers/duties/Odin.java +++ b/triggers/triggers-general/src/main/java/gg/xp/xivsupport/events/triggers/duties/Odin.java @@ -12,6 +12,7 @@ /** * Example trigger pack for a duty */ +// TODO: this is very outdated // @CalloutRepo indicates that the system should scan for fields defined as ModifiableCallout. The user is presented // with a UI to enable/disable them, and change the callout text under the Plugins > Callouts tab. // The name chosen here will show in the UI. @@ -24,11 +25,17 @@ public class Odin implements FilteredEventHandler { // Since we have @CalloutRepo private final ModifiableCallout valknut = ModifiableCallout.durationBasedCall("Valknut (Out)", "Out"); + private final XivState state; + + public Odin(XivState state) { + this.state = state; + } + // This comes from FilteredEventHandler. In this case, we want to restrict this set of triggers to a specific // zone (Urth's Fount, in this case, Zone ID 394). @Override public boolean enabled(EventContext context) { - return context.getStateInfo().get(XivState.class).zoneIs(394); + return state.zoneIs(394); } // This is an actual callout. You can specify as many as you want, but you have to follow the usual Java conventions diff --git a/triggers/triggers-sb/src/main/java/gg/xp/xivsupport/events/triggers/duties/UCoB.java b/triggers/triggers-sb/src/main/java/gg/xp/xivsupport/events/triggers/duties/UCoB.java index 534422d4b2f7..15bfdc832bef 100644 --- a/triggers/triggers-sb/src/main/java/gg/xp/xivsupport/events/triggers/duties/UCoB.java +++ b/triggers/triggers-sb/src/main/java/gg/xp/xivsupport/events/triggers/duties/UCoB.java @@ -66,7 +66,7 @@ public UCoB(XivState state) { // Todo: add triggers for Akh morn and Morn Afah @Override public boolean enabled(EventContext context) { - return context.getStateInfo().get(XivStateImpl.class).zoneIs(0x2DD); + return state.dutyIs(KnownDuty.UCoB); } // @HandleEvents diff --git a/xivsupport/src/main/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLock.java b/xivsupport/src/main/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLock.java index 872fa8caef82..74dbe1618d93 100644 --- a/xivsupport/src/main/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLock.java +++ b/xivsupport/src/main/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLock.java @@ -3,16 +3,25 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; -public class EnhancedReadWriteReentrantLock { +public final class EnhancedReadWriteReentrantLock { - private final ReadWriteLock lock = new ReentrantReadWriteLock(); + private final LockAdapter readAdp; + private final LockAdapter writeAdp; + + public EnhancedReadWriteReentrantLock() { + ReadWriteLock lock = new ReentrantReadWriteLock(); + readAdp = new LockAdapter(lock.readLock()); + writeAdp = new LockAdapter(lock.writeLock()); + } public LockAdapter read() { - return new LockAdapter(lock.readLock()); + readAdp.lock(); + return readAdp; } public LockAdapter write() { - return new LockAdapter(lock.writeLock()); + writeAdp.lock(); + return writeAdp; } } diff --git a/xivsupport/src/main/java/gg/xp/xivsupport/sys/LockAdapter.java b/xivsupport/src/main/java/gg/xp/xivsupport/sys/LockAdapter.java index dc63f7259a36..5c2acd1923ac 100644 --- a/xivsupport/src/main/java/gg/xp/xivsupport/sys/LockAdapter.java +++ b/xivsupport/src/main/java/gg/xp/xivsupport/sys/LockAdapter.java @@ -8,10 +8,13 @@ public class LockAdapter implements AutoCloseable { private final Lock lock; public LockAdapter(Lock lock) { - lock.lock(); this.lock = lock; } + void lock() { + lock.lock(); + } + @Override public void close() { lock.unlock(); From f2a041784a0104975297b24582e2cab0efdd8cb0 Mon Sep 17 00:00:00 2001 From: XP Date: Fri, 16 Jan 2026 16:15:12 -0800 Subject: [PATCH 2/2] Add unit test --- .../EnhancedReadWriteReentrantLockTest.java | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 xivsupport/src/test/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLockTest.java diff --git a/xivsupport/src/test/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLockTest.java b/xivsupport/src/test/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLockTest.java new file mode 100644 index 000000000000..98375ac25782 --- /dev/null +++ b/xivsupport/src/test/java/gg/xp/xivsupport/sys/EnhancedReadWriteReentrantLockTest.java @@ -0,0 +1,162 @@ +package gg.xp.xivsupport.sys; + +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; + +public class EnhancedReadWriteReentrantLockTest { + + /** + * Test that a read lock can be acquired and released using try-with-resources. + */ + @Test + public void testReadLock() { + EnhancedReadWriteReentrantLock lock = new EnhancedReadWriteReentrantLock(); + AtomicBoolean locked = new AtomicBoolean(false); + try (LockAdapter ignored = lock.read()) { + locked.set(true); + } + MatcherAssert.assertThat(locked.get(), Matchers.is(true)); + } + + /** + * Test that a write lock can be acquired and released using try-with-resources. + */ + @Test + public void testWriteLock() { + EnhancedReadWriteReentrantLock lock = new EnhancedReadWriteReentrantLock(); + AtomicBoolean locked = new AtomicBoolean(false); + try (LockAdapter ignored = lock.write()) { + locked.set(true); + } + MatcherAssert.assertThat(locked.get(), Matchers.is(true)); + } + + /** + * Test that multiple read locks can be held simultaneously by the same thread. + */ + @Test + public void testMultipleReadLocks() { + EnhancedReadWriteReentrantLock lock = new EnhancedReadWriteReentrantLock(); + try (LockAdapter ignored1 = lock.read()) { + try (LockAdapter ignored2 = lock.read()) { + // Should not block + } + } + } + + /** + * Test that a write lock is exclusive and blocks other write lock attempts from different threads. + * + * @throws InterruptedException if the wait is interrupted + * @throws ExecutionException if the background task fails + * @throws TimeoutException if the timeout expires + */ + @Test + public void testWriteLockExclusivity() throws InterruptedException, ExecutionException, TimeoutException { + EnhancedReadWriteReentrantLock lock = new EnhancedReadWriteReentrantLock(); + AtomicBoolean writeLocked = new AtomicBoolean(false); + CompletableFuture secondLockAcquired = new CompletableFuture<>(); + + try (LockAdapter ignored = lock.write()) { + writeLocked.set(true); + CompletableFuture.runAsync(() -> { + try (LockAdapter ignored2 = lock.write()) { + secondLockAcquired.complete(null); + } + }); + + try { + secondLockAcquired.get(100, TimeUnit.MILLISECONDS); + Assert.fail("Second write lock should have been blocked"); + } catch (TimeoutException e) { + // Expected + } + } + + secondLockAcquired.get(1, TimeUnit.SECONDS); + } + + /** + * Test that a write lock blocks read lock attempts from different threads. + * + * @throws InterruptedException if the wait is interrupted + * @throws ExecutionException if the background task fails + * @throws TimeoutException if the timeout expires + */ + @Test + public void testReadWriteExclusivity() throws InterruptedException, ExecutionException, TimeoutException { + EnhancedReadWriteReentrantLock lock = new EnhancedReadWriteReentrantLock(); + CompletableFuture readLockAcquired = new CompletableFuture<>(); + + try (LockAdapter ignored = lock.write()) { + CompletableFuture.runAsync(() -> { + try (LockAdapter ignored2 = lock.read()) { + readLockAcquired.complete(null); + } + }); + + try { + readLockAcquired.get(100, TimeUnit.MILLISECONDS); + Assert.fail("Read lock should have been blocked by write lock"); + } catch (TimeoutException e) { + // Expected + } + } + + readLockAcquired.get(1, TimeUnit.SECONDS); + } + + /** + * Test that a read lock blocks write lock attempts from different threads. + * + * @throws InterruptedException if the wait is interrupted + * @throws ExecutionException if the background task fails + * @throws TimeoutException if the timeout expires + */ + @Test + public void testWriteAfterReadExclusivity() throws InterruptedException, ExecutionException, TimeoutException { + EnhancedReadWriteReentrantLock lock = new EnhancedReadWriteReentrantLock(); + CompletableFuture writeLockAcquired = new CompletableFuture<>(); + + try (LockAdapter ignored = lock.read()) { + CompletableFuture.runAsync(() -> { + try (LockAdapter ignored2 = lock.write()) { + writeLockAcquired.complete(null); + } + }); + + try { + writeLockAcquired.get(100, TimeUnit.MILLISECONDS); + Assert.fail("Write lock should have been blocked by read lock"); + } catch (TimeoutException e) { + // Expected + } + } + + writeLockAcquired.get(1, TimeUnit.SECONDS); + } + + /** + * Test the reentrant behavior of the lock, specifically acquiring a write lock + * multiple times and then a read lock within the same thread. + */ + @Test + public void testReentrancy() { + EnhancedReadWriteReentrantLock lock = new EnhancedReadWriteReentrantLock(); + try (LockAdapter ignored1 = lock.write()) { + try (LockAdapter ignored2 = lock.write()) { + try (LockAdapter ignored3 = lock.read()) { + // All good + } + } + } + } +}