Hacktoberfest 2026:维护者为十月标记出来的 issue,仍然开放、适合新手。 浏览 Hacktoberfest issue

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

未关闭
#119 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

评估

难度
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

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

ome/ome-common-java 的其他 Issue

查看 ome/ome-common-java 的全部 Issue

相似的 Issue

更多 Java Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。