Hacktoberfest 2026:メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

Reconsidering #78: `NIOFileHandle` extends the file on every write past EOF

オープン
#119 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
45/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
静か
技術スタック
java

調査の方向性

まず AbstractNIOHandle.validateLength と NIOFileHandle.setLength/doWrite を読み、次に RandomAccessInputStream のバッファリングと TiffSaver.writeIFD/writeIFDValue の呼び出しパターンを調べてください。コーディングする前に、提案されているどの対処方法をメンテナーが受け入れるか確認してください。完了の条件は、書き込み長とファイル内容の正確性をカバーする、合意済みの実装です。

索引モデルが issue の本文から書いたものです。

説明

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:

  • #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 ome/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.

In loci/common/AbstractNIOHandle.java:

protected boolean validateLength(int writeLength) throws IOException {
  if (getFilePointer() + writeLength > length()) {
    setLength(getFilePointer() + writeLength);
    return false;
  }
  return true;
}

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.

import loci.common.NIOFileHandle;

import java.io.File;
import java.io.RandomAccessFile;
import java.util.concurrent.CountDownLatch;

public class NIOFileHandleAppendBenchmark {

  private static final int WRITES = 200_000;

  public static void main(String[] args) throws Exception {
    File dir = new File(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 (int threads : new int[] { 1, 4, 8, 20 }) {
      run(dir, threads, "NIOFileHandle, appending", true);
      run(dir, threads, "RandomAccessFile, pre-sized", false);
    }
  }

  private static void run(File dir, int nThreads, String label, boolean useHandle) throws Exception {
    Thread[] threads = new Thread[nThreads];
    CountDownLatch start = new CountDownLatch(1);
    long[] nanos = new long[nThreads];
    for (int i = 0; i < nThreads; i++) {
      final int id = i;
      threads[i] = new Thread(() -> {
        File file = new File(dir, "bench_" + (useHandle ? "nio" : "raf") + "_" + nThreads + "_" + id + ".bin");
        file.delete();
        try {
          start.await();
          long t0 = System.nanoTime();
          if (useHandle) appendThroughHandle(file);
          else appendPreSized(file);
          nanos[id] = System.nanoTime() - t0;
        }
        catch (Exception e) {
          e.printStackTrace();
        }
        file.delete();
      });
      threads[i].start();
    }
    start.countDown();
    for (Thread t : threads) t.join();

    long total = 0;
    for (long n : nanos) total += n;
    double usPerWrite = (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 */
  private static void appendThroughHandle(File file) throws Exception {
    NIOFileHandle handle = new NIOFileHandle(file, "rw");
    for (int i = 0; i < WRITES; i++) handle.writeShort(i);
    handle.close();
  }

  /** The same writes, on a file that already has its final length */
  private static void appendPreSized(File file) throws Exception {
    RandomAccessFile raf = new RandomAccessFile(file, "rw");
    raf.setLength(2L * WRITES);
    for (int i = 0; i < WRITES; i++) raf.writeShort(i);
    raf.close();
  }
}

Output here (Windows 11, NTFS, NVMe SSD, 32 logical cores, JDK 21.0.11, ome-common 6.2.1):

threads   handle                             us/write   writes/s total
1         NIOFileHandle, appending              27.26            36683
1         RandomAccessFile, pre-sized            1.85           541524
4         NIOFileHandle, appending              65.29            61269
4         RandomAccessFile, pre-sized            3.00          1332104
8         NIOFileHandle, appending             151.06            52959
8         RandomAccessFile, pre-sized            3.25          2459027
20        NIOFileHandle, appending             481.17            41566
20        RandomAccessFile, pre-sized            5.17          3866420

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:

private void doWrite(int length) throws IOException {
  buffer.position(buffer.position() - length);
  channel.write(buffer, position);
  position += length;
}

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
  • Windows 11, NTFS, NVMe SSD, 32 logical cores
  • JDK 21.0.11
  • Seen from https://github.com/BIOP/ijp-kheops (pyramidal OME-TIFF export)

(Disclaimer: AI assisted)

主要言語
Java
スター
2
フォーク
19
PR マージ指標
30日以内にマージされた PR はありません

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

ome/ome-common-java のほかの issue

ome/ome-common-java の issue をすべて見る

似ている issue

Java の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。