You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is the same problem as #78 (Add write buffering in RandomAccessOutputStream), which was opened in June 2023 and closed as not planned in November 2024. The diagnosis there is already correct and the remedies proposed in it are the right ones, I am not bringing a new theory, only measurements and one new fact.
What I think is worth a second look:
Add write buffering in RandomAccessOutputStream #78 was based on reports that repeated setLength calls "cause a significant slowdown", without a figure attached. Below is a standalone reproducer and a number: ~15x per write, and, the part I did not expect, the aggregate rate does not improve with more threads at all, so writing several files in parallel does not work around it.
The related Bio-Formats PR Performance increase in writeIFD bioformats#3680, which buffered the IFD through a ByteArrayHandle, was closed without being merged, so nothing landed on that side either.
As of v6.3.0 the code is unchanged (v6.2.1...v6.3.0 touches only CI, README, pom and a test resource), so this is still current behaviour and not something already improved that I am misreading.
A downstream workaround turns out not to be possible, for a specific reason to do with doWrite that I detail at the end. That is the one genuinely new piece of information here, and it is why I am asking rather than just fixing it on our side.
If #78 was closed on a risk/benefit judgement rather than on the merits, the numbers below may shift the benefit side, and the last section proposes a shape that keeps today's behaviour as the default. If it was closed for a reason I cannot see from the outside, please just say so and I will stop pushing.
Summary
AbstractNIOHandle.validateLength calls setLength on every write that goes past the end of the file, and NIOFileHandle.setLength turns that into a RandomAccessFile.setLength system call plus a discarded NIO buffer.
For code that appends in large blocks this is irrelevant. For code that appends many small values it is the dominant cost, because each 2-byte write becomes a file-system metadata operation. TiffSaver.writeIFD / writeIFDValue write an image directory field by field (writeShort, writeInt, writeLong), so a single IFD costs on the order of a hundred setLength calls.
We hit this writing pyramidal OME-TIFFs from a Leica LIF with many small planes (84 images x 90 z x 2 channels = 15 120 IFDs, ~1.5 M extending writes). A sampling profiler over the 58 s export attributes ~710 thread-seconds to RandomAccessFile.setLength0 under TiffSaver.writeIFD, against 30 for LZW compression and ~1 for decoding the input. Roughly 70 % of the wall time is spent extending the output file two bytes at a time.
Reproducer
Only depends on ome-common (plus slf4j-api on the classpath). Each thread writes to its own file, so nothing is shared between them.
importloci.common.NIOFileHandle;
importjava.io.File;
importjava.io.RandomAccessFile;
importjava.util.concurrent.CountDownLatch;
publicclassNIOFileHandleAppendBenchmark {
privatestaticfinalintWRITES = 200_000;
publicstaticvoidmain(String[] args) throwsException {
Filedir = newFile(args.length > 0 ? args[0] : System.getProperty("java.io.tmpdir"));
dir.mkdirs();
System.out.println(System.getProperty("os.name") + ", java "
+ System.getProperty("java.version") + ", " + WRITES
+ " two-byte writes per thread, in " + dir);
System.out.printf("%n%-8s %-30s %12s %16s%n", "threads", "handle", "us/write", "writes/s total");
for (intthreads : newint[] { 1, 4, 8, 20 }) {
run(dir, threads, "NIOFileHandle, appending", true);
run(dir, threads, "RandomAccessFile, pre-sized", false);
}
}
privatestaticvoidrun(Filedir, intnThreads, Stringlabel, booleanuseHandle) throwsException {
Thread[] threads = newThread[nThreads];
CountDownLatchstart = newCountDownLatch(1);
long[] nanos = newlong[nThreads];
for (inti = 0; i < nThreads; i++) {
finalintid = i;
threads[i] = newThread(() -> {
Filefile = newFile(dir, "bench_" + (useHandle ? "nio" : "raf") + "_" + nThreads + "_" + id + ".bin");
file.delete();
try {
start.await();
longt0 = System.nanoTime();
if (useHandle) appendThroughHandle(file);
elseappendPreSized(file);
nanos[id] = System.nanoTime() - t0;
}
catch (Exceptione) {
e.printStackTrace();
}
file.delete();
});
threads[i].start();
}
start.countDown();
for (Threadt : threads) t.join();
longtotal = 0;
for (longn : nanos) total += n;
doubleusPerWrite = (total / (double) nThreads) / WRITES / 1000.0;
System.out.printf("%-8d %-30s %12.2f %16.0f%n", nThreads, label, usPerWrite,
nThreads * 1e6 / usPerWrite);
}
/** What TiffSaver does when it writes an IFD field by field */privatestaticvoidappendThroughHandle(Filefile) throwsException {
NIOFileHandlehandle = newNIOFileHandle(file, "rw");
for (inti = 0; i < WRITES; i++) handle.writeShort(i);
handle.close();
}
/** The same writes, on a file that already has its final length */privatestaticvoidappendPreSized(Filefile) throwsException {
RandomAccessFileraf = newRandomAccessFile(file, "rw");
raf.setLength(2L * WRITES);
for (inti = 0; i < WRITES; i++) raf.writeShort(i);
raf.close();
}
}
Two things: ~15x per write single-threaded, and the aggregate throughput does not scale with threads (37 k/s at 1 thread, 42 k/s at 20), because file extension serializes in the file system. Writing several OME-TIFFs in parallel therefore does not help.
I have only measured this on Windows/NTFS. The magnitude is very likely OS- and file-system-dependent, and I would not be surprised if it is much smaller on Linux.
Why this cannot be worked around downstream
We tried, with a NIOFileHandle subclass installed through Location.mapFile that keeps the file physically larger than its content and reports the content length. It gives about a 3x speed-up on the export above, but we could not make it correct, for a reason worth recording:
doWrite writes everything from the buffer position to the buffer limit, not just the bytes the caller asked for:
so the file can end further than setLength was ever told — 84 bytes per plane in our case. The stock class never notices, because length() just returns raf.length(). A subclass that tracks a logical length has no way to observe that end: reading bufferStartPosition + buffer.limit() lazily in length() over-estimates, and reading it after each overridden write* under-estimates. The true value is only known inside doWrite, which is private.
In other words the fix is straightforward inside the class and not reachable from outside it — which is why I am asking here instead of keeping this in our own code.
Possible fix, and the side effects
Both remedies proposed in #78 would work. They differ in what they change on disk:
Option A: buffer the writes (#78, second bullet: a reusable ByteBuffer of configurable size, as RandomAccessInputStream already does for reads). This is the one I would favour, and it is worth noting explicitly that it has no on-disk side effect at all: the file never becomes longer than its content, length() semantics are untouched, and nothing changes for any other caller. It only moves when bytes reach the OS, which for a file being written through a single handle is not observable. This is also what ome/bioformats#3680 was doing at the TIFF level, and it addresses the third bullet of #78 (what close() should do about the length) by never creating the discrepancy in the first place.
Option B: grow the file in chunks / allow an initial length (#78, first bullet). Simpler, but it does have a visible consequence, and I would rather name it than have it discovered later:
length() must keep returning the logical content length, not the padded size, or every TIFF offset computed from it moves. That is the whole correctness question, and it is what doWrite above makes delicate.
A file being written would be larger on disk than its content until it is closed. Another handle on the same path, an external process, or a crash would see trailing padding. Today that cannot happen.
close() would perform a truncation, so it can do I/O and fail where it previously could not.
Read-only handles are unaffected; only "rw" mode is concerned.
If Option B is considered at all, the safe shape is opt-in — a growth increment defaulting to 0, i.e. today's semantics — so nothing changes for existing callers unless they ask for it.
I am happy to put together a PR with tests for Option A if that direction is acceptable. I would rather agree on the approach first than send code at a closed issue.
Environment
ome-common 6.2.1 (behaviour unchanged in 6.3.0), Bio-Formats 8.5.0
Why I am raising this again
This is the same problem as #78 (Add write buffering in RandomAccessOutputStream), which was opened in June 2023 and closed as not planned in November 2024. The diagnosis there is already correct and the remedies proposed in it are the right ones, I am not bringing a new theory, only measurements and one new fact.
What I think is worth a second look:
setLengthcalls "cause a significant slowdown", without a figure attached. Below is a standalone reproducer and a number: ~15x per write, and, the part I did not expect, the aggregate rate does not improve with more threads at all, so writing several files in parallel does not work around it.ByteArrayHandle, was closed without being merged, so nothing landed on that side either.v6.2.1...v6.3.0touches only CI, README, pom and a test resource), so this is still current behaviour and not something already improved that I am misreading.doWritethat I detail at the end. That is the one genuinely new piece of information here, and it is why I am asking rather than just fixing it on our side.If #78 was closed on a risk/benefit judgement rather than on the merits, the numbers below may shift the benefit side, and the last section proposes a shape that keeps today's behaviour as the default. If it was closed for a reason I cannot see from the outside, please just say so and I will stop pushing.
Summary
AbstractNIOHandle.validateLengthcallssetLengthon every write that goes past the end of the file, andNIOFileHandle.setLengthturns that into aRandomAccessFile.setLengthsystem call plus a discarded NIO buffer.In
loci/common/AbstractNIOHandle.java:For code that appends in large blocks this is irrelevant. For code that appends many small values it is the dominant cost, because each 2-byte write becomes a file-system metadata operation.
TiffSaver.writeIFD/writeIFDValuewrite an image directory field by field (writeShort,writeInt,writeLong), so a single IFD costs on the order of a hundredsetLengthcalls.We hit this writing pyramidal OME-TIFFs from a Leica LIF with many small planes (84 images x 90 z x 2 channels = 15 120 IFDs, ~1.5 M extending writes). A sampling profiler over the 58 s export attributes ~710 thread-seconds to
RandomAccessFile.setLength0underTiffSaver.writeIFD, against 30 for LZW compression and ~1 for decoding the input. Roughly 70 % of the wall time is spent extending the output file two bytes at a time.Reproducer
Only depends on
ome-common(plusslf4j-apion the classpath). Each thread writes to its own file, so nothing is shared between them.Output here (Windows 11, NTFS, NVMe SSD, 32 logical cores, JDK 21.0.11, ome-common 6.2.1):
Two things: ~15x per write single-threaded, and the aggregate throughput does not scale with threads (37 k/s at 1 thread, 42 k/s at 20), because file extension serializes in the file system. Writing several OME-TIFFs in parallel therefore does not help.
I have only measured this on Windows/NTFS. The magnitude is very likely OS- and file-system-dependent, and I would not be surprised if it is much smaller on Linux.
Why this cannot be worked around downstream
We tried, with a
NIOFileHandlesubclass installed throughLocation.mapFilethat keeps the file physically larger than its content and reports the content length. It gives about a 3x speed-up on the export above, but we could not make it correct, for a reason worth recording:doWritewrites everything from the buffer position to the buffer limit, not just the bytes the caller asked for:so the file can end further than
setLengthwas ever told — 84 bytes per plane in our case. The stock class never notices, becauselength()just returnsraf.length(). A subclass that tracks a logical length has no way to observe that end: readingbufferStartPosition + buffer.limit()lazily inlength()over-estimates, and reading it after each overriddenwrite*under-estimates. The true value is only known insidedoWrite, which is private.In other words the fix is straightforward inside the class and not reachable from outside it — which is why I am asking here instead of keeping this in our own code.
Possible fix, and the side effects
Both remedies proposed in #78 would work. They differ in what they change on disk:
Option A: buffer the writes (#78, second bullet: a reusable
ByteBufferof configurable size, asRandomAccessInputStreamalready does for reads). This is the one I would favour, and it is worth noting explicitly that it has no on-disk side effect at all: the file never becomes longer than its content,length()semantics are untouched, and nothing changes for any other caller. It only moves when bytes reach the OS, which for a file being written through a single handle is not observable. This is also what ome/bioformats#3680 was doing at the TIFF level, and it addresses the third bullet of #78 (whatclose()should do about the length) by never creating the discrepancy in the first place.Option B: grow the file in chunks / allow an initial length (#78, first bullet). Simpler, but it does have a visible consequence, and I would rather name it than have it discovered later:
length()must keep returning the logical content length, not the padded size, or every TIFF offset computed from it moves. That is the whole correctness question, and it is whatdoWriteabove makes delicate.close()would perform a truncation, so it can do I/O and fail where it previously could not."rw"mode is concerned.If Option B is considered at all, the safe shape is opt-in — a growth increment defaulting to 0, i.e. today's semantics — so nothing changes for existing callers unless they ask for it.
I am happy to put together a PR with tests for Option A if that direction is acceptable. I would rather agree on the approach first than send code at a closed issue.
Environment
(Disclaimer: AI assisted)