denis-chudov commented on code in PR #7970:
URL: https://github.com/apache/ignite-3/pull/7970#discussion_r3093632697
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java:
##########
@@ -903,6 +917,52 @@ public int waitersCount() {
}
}
+ /**
+ * Checks if any currently held lock mode is incompatible with the
intended mode.
+ * Runs in O(1) time (constant number of lock modes). Must be called
under {@code synchronized (waiters)}.
+ */
+ private boolean hasAnyIncompatibleHolder(LockMode intended) {
+ for (LockMode held : LOCK_MODES) {
+ if (lockModeHeldCount[held.ordinal()] > 0 &&
!held.isCompatible(intended)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /** Adjusts {@code lockModeHeldCount} for a mode transition. Must be
called under {@code synchronized (waiters)}. */
+ private void updateHeldCount(@Nullable LockMode oldMode, @Nullable
LockMode newMode) {
+ if (oldMode == newMode) {
+ return;
+ }
+ if (oldMode != null) {
+ lockModeHeldCount[oldMode.ordinal()]--;
+ }
+ if (newMode != null) {
+ lockModeHeldCount[newMode.ordinal()]++;
+ }
+ }
+
+ /** Locks the waiter and updates {@code lockModeHeldCount}. Must be
called under {@code synchronized (waiters)}. */
+ private void lockOnWaiter(WaiterImpl waiter) {
+ LockMode oldMode = waiter.lockMode();
+ waiter.lock();
+ updateHeldCount(oldMode, waiter.lockMode());
+ }
+
+ /** Validates lockModeHeldCount is consistent with actual waiter
state. Must be called under {@code synchronized (waiters)}. */
+ private void assertLockModeHeldCount() {
+ int[] expected = new int[LOCK_MODES.length];
+ for (WaiterImpl w : waiters.values()) {
+ if (w.lockMode != null) {
+ expected[w.lockMode.ordinal()]++;
+ }
+ }
Review Comment:
Are you sure we want to check this every time?
Did you run benchmarks?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java:
##########
@@ -914,6 +974,14 @@ private boolean isWaiterReadyToNotify(WaiterImpl waiter,
boolean skipFail) {
assert intendedLockMode != null : "Intended lock mode is null";
+ // Fast path: no incompatible holders — grant immediately.
+ // Conservative for upgrades: the waiter's own mode is in the
counts, causing a fallthrough to the slow path.
+ if (!hasAnyIncompatibleHolder(intendedLockMode)) {
Review Comment:
is it correct that the main idea is adding fast path for
`isWaiterReadyToNotify` to reduce the time which tread spends inside
`synchronized`?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java:
##########
@@ -903,6 +917,52 @@ public int waitersCount() {
}
}
+ /**
+ * Checks if any currently held lock mode is incompatible with the
intended mode.
+ * Runs in O(1) time (constant number of lock modes). Must be called
under {@code synchronized (waiters)}.
+ */
+ private boolean hasAnyIncompatibleHolder(LockMode intended) {
+ for (LockMode held : LOCK_MODES) {
+ if (lockModeHeldCount[held.ordinal()] > 0 &&
!held.isCompatible(intended)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /** Adjusts {@code lockModeHeldCount} for a mode transition. Must be
called under {@code synchronized (waiters)}. */
+ private void updateHeldCount(@Nullable LockMode oldMode, @Nullable
LockMode newMode) {
+ if (oldMode == newMode) {
+ return;
+ }
+ if (oldMode != null) {
+ lockModeHeldCount[oldMode.ordinal()]--;
+ }
+ if (newMode != null) {
+ lockModeHeldCount[newMode.ordinal()]++;
+ }
+ }
+
+ /** Locks the waiter and updates {@code lockModeHeldCount}. Must be
called under {@code synchronized (waiters)}. */
+ private void lockOnWaiter(WaiterImpl waiter) {
+ LockMode oldMode = waiter.lockMode();
+ waiter.lock();
+ updateHeldCount(oldMode, waiter.lockMode());
+ }
+
+ /** Validates lockModeHeldCount is consistent with actual waiter
state. Must be called under {@code synchronized (waiters)}. */
+ private void assertLockModeHeldCount() {
+ int[] expected = new int[LOCK_MODES.length];
+ for (WaiterImpl w : waiters.values()) {
+ if (w.lockMode != null) {
+ expected[w.lockMode.ordinal()]++;
+ }
+ }
+ assert Arrays.equals(expected, lockModeHeldCount)
Review Comment:
As I remember, we didn't want to use `assert` where it's not necessary, is
it still so?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
