Issue 61 Fix with old style "Access" instead of "Rental" #84

eemhu · 2024-03-01T14:29:11Z

Add new phaser implementation rather than using a custom count and lock solution to BufferLease and Access.

…se> and Supplier<Lease>.

…re than 1 instead of zero as Phaser inits at 1, not 0.

…rk BufferLeaseImpl.refs as unreliable, change Rental.closed() to return phaser.isTerminated

Note: RentalTest contains IllegalStateException assert that does not seem to happen, commented out for now.

… with the pre-existing buffer and id.

…rnalOffer, add stub unless phaser was terminated explicitly

… to queue.

…Lease to BufferContainer. Pool holds BufferContainers and wraps them in BufferLeases while out of the queue.

… to fix dirty buffer in pool.

# Conflicts: # src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeaseImpl.java # src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeasePool.java

src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeaseImpl.java

src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeasePool.java

src/main/java/com/teragrep/rlp_03/context/frame/access/Access.java

StrongestNumber9 · 2024-03-05T11:18:28Z

General note: Noticed mixed use of this.classVariable and classVariable but didn't comment on all of them. Use them consistently in one way or another, we usually refer to this.classVariable only inside constructor and classVariable everywhere else

src/main/java/com/teragrep/rlp_03/context/buffer/BufferContainerImpl.java

StrongestNumber9 · 2024-03-05T11:26:26Z

src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeasePool.java

-                        BufferLease queuedBufferLease = queue.poll();
-                        if (queuedBufferLease == null) {
+                        BufferContainer queuedBufferContainer = queue.poll();
+                        if (queuedBufferContainer == null) {
                            break;
                        }
                    }


Will this implementation consume all available processing power in a busy loop until poll returns null?

…ion, rename access.terminated() to isTerminated(), fix BufferContainerImpl logger class, remove attemptRelease() from BufferLease interface, remove usage of 'this' keyword outside of constructors, inline phaser (de)register output in BufferLeaseImpl and throw IllegalStateException in case of dirty pool,

src/main/java/com/teragrep/rlp_03/context/frame/access/Lease.java

eemhu · 2024-03-05T11:28:45Z

added fixes till "wrong classname" comment

src/main/java/com/teragrep/rlp_03/context/frame/access/Lease.java

…e() if-else clause.

…tead of peek() and poll() until null

eemhu · 2024-03-05T14:02:35Z

added fixes to rest of the comments.

kortemik

move Access stuff to it's own pr and this will do

src/main/java/com/teragrep/rlp_03/context/buffer/BufferLease.java

src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeaseImpl.java

src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeasePool.java

src/main/java/com/teragrep/rlp_03/context/frame/access/Access.java

src/main/java/com/teragrep/rlp_03/context/frame/access/Lease.java

…easePool

…equest.

eemhu · 2024-03-06T10:40:31Z

@kortemik Reverted to main branch Access / Lease / AccessTest. This PR should now only contain the changed BufferLease, will move Access phaser to new pull request.

StrongestNumber9

LGTM

src/main/java/com/teragrep/rlp_03/context/buffer/BufferContainerImpl.java

… class name

eemhu and others added 20 commits February 19, 2024 15:54

add phaser to Access.java

ef7cb18

remove arriveAndAwaitAdvance calls, add phaser to BufferLeaseImpl.

2882b0e

rename Access class to Rental. Implements AutoCloseable, Consumer<Lea…

f8267cf

…se> and Supplier<Lease>.

more renames from 'access' to 'rental'. check if registrations are mo…

08e15e8

…re than 1 instead of zero as Phaser inits at 1, not 0.

change BufferLeaseImpl.isRefCountZero to check phaser termination, ma…

334d130

…rk BufferLeaseImpl.refs as unreliable, change Rental.closed() to return phaser.isTerminated

fix attemptRelease condition.

38ec3d0

Note: RentalTest contains IllegalStateException assert that does not seem to happen, commented out for now.

add subPhaser to Lease.java

bba2af2

re-create BufferLeaseImpl in BufferLeasePool if phaser was terminated…

61b152a

… with the pre-existing buffer and id.

add PhaserDecoratedBufferLease and in BufferLeasePool use it. In Inte…

8169cb3

…rnalOffer, add stub unless phaser was terminated explicitly

remove comparisons from phaser. unwrap phaser decorator before adding…

9100506

… to queue.

add synchronized keyword to BufferLeaseImpl.buffer() to get rid of lock

1f422d9

replace instanceof check with isPhaserDecorated method

0cceb85

Change PhaserDecoratedBufferLease to be BufferLease and change Buffer…

9a6e836

…Lease to BufferContainer. Pool holds BufferContainers and wraps them in BufferLeases while out of the queue.

fix wrong logging in BufferLeasePool.take()

68097c6

do not add empty stub buffers to pool

0047aa7

FIXME termination of dirty state

a1534ed

add 'synchronized' keyword to BufferLeaseImpl.attemptRelease() method…

e7c40c7

… to fix dirty buffer in pool.

revert to Access for this branch

78640ba

Merge branch 'main' into issue61-withaccess

c9b82a4

# Conflicts: # src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeaseImpl.java # src/main/java/com/teragrep/rlp_03/context/buffer/BufferLeasePool.java

revert missed rental references back to access

47bbc44

eemhu marked this pull request as ready for review March 4, 2024 07:23

add ClearingPhaser from @kortemik/clearing-phaser

a7d0927

eemhu requested a review from kortemik March 4, 2024 08:56

add more descriptive exceptions to BufferLeaseImpl

b9dc0a3