Skip to content

FileDataBlock experiment - #1

Draft
icikle wants to merge 3 commits into
fix/gh-51463-nested-jar-lockingfrom
fix/gh-52463-filedatablock-experiment
Draft

icikle wants to merge 3 commits into
fix/gh-51463-nested-jar-lockingfrom
fix/gh-52463-filedatablock-experiment

Conversation

@icikle

@icikle icikle commented Sep 16, 2026

Copy link
Copy Markdown
Owner

This PR is looking at how to extend spring-projects#51580 so that the proof app starts up without locking up. Note that spring-projects#51580 already eliminates the hasEntry as a place where the threads get blocked. With that change though the locking hit further into the FileDataBlock or NestedJarFile. This branch does 2 things to minimise this blocking.

  • Moves FileDataBlock.FileAccess to use a StampedLock rather than synchronizing on an object mutex. The StampedLock suits well here and as the reference count mechanism in the class doesn't fit well with a read write lock. The StampedLock allows for optimistic reading of the block, if the optomistic read is not suitable it attempts to get a write lock with a timeout. If the write lock is obtained the buffer is filled and if the write lock can't be obtained its recursing back into the method to try the read again. This would need more safety to ensure it doesn't continue looping.
  • Moves the synchronization in JarEntriesEnumeration#nextElement to be on the JarEntriesEnumeration instance rather than synchronising on the parent NestedJarFile instance. The change here will mean that 2 JarEntriesEnumeration instances for the same NestedJarFile instances will be not contest for a lock on the same object. From my initial investigation the zipContent.getEntry is safe to be used in this way.

My other idea reducing contention in the FileDataBlock.FileAccess is more involved and possibly overkill if contention is rare.

Note - ignore the issue number in the branch name - it has a typo in it

Signed-off-by: Ian Kettle <25729118+icikle@users.noreply.github.com>
…riesEnumeration to be on the instance rather than the parent NestedJarFile.

Signed-off-by: Ian Kettle <25729118+icikle@users.noreply.github.com>
…riesEnumeration to be on the instance rather than the parent NestedJarFile.

Signed-off-by: Ian Kettle <25729118+icikle@users.noreply.github.com>
return new NestedJarEntry(this.zipContent.getEntry(this.cursor++));
int nextValue;
ZipContent zipContent = ensureOpen();
synchronized (this) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

synchronizing on the JarEntriesEnumeration rather than the NestedJarFile instance. Means that 2 JarEntriesEnumeration instances won't contest for the same lock. Moving the zipContent.getEntry out of the synchronized block appears safe. I would investigate more before saying is definetly safe.

long writeStamp = 0;
try {
// any smaller and the scheduler and the parking wouldn't be useful and recursion too immediate.
writeStamp = this.lock.tryWriteLock(2, TimeUnit.MILLISECONDS);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tries to get a write lock and if not obtained in 2ms it recurses back to try the optimistic read again. Would want additional saftey on the recursion to ensure it can't result in a stack overflow.

@icikle

icikle commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

This is a dump from the proof app running with this branches changes.

sixth-dump.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant