Image not written to disk

[andreialecu]

We have been trying docunotion and ran into an issue where an image that was being downloaded was not making it to the disk correctly.

I tracked it down to this line:

docu-notion/src/images.ts

Line 204 in f1176c5

fs.createWriteStream(path).write(buffer); // async but we're not waiting

Changing it to fs.writeFileSync(path, buffer) fixes the issue.

It's unclear why the code does not await for the file to be written, is there any reason for that?

[hatton]

Yeah, because it allows more to be happening in parallel. What issue did you encounter?

[andreialecu]

There was an image that would not make it to disk.

It would simply be missing, and then the docusaurus build would fail with a broken link to it. The documentation would properly reference the file, just that it wasn't there.

I assume the reason is that the process exists before all files get a chance to be written. It was always the last image written.

Regarding the optimization: it seems odd that writing files on modern SSD drives would need this, and it would make any difference.

[hatton]

Regarding the optimization: it seems odd that writing files on modern SSD drives would need this, and it would make any difference.

I haven't profiled this. But in theory, it should make a difference: SSDs are faster than mechanical drives, but still very, very slow compared to code. After all, the premise here is that the problem you encountered is that the async call failed to finish in time and so it didn't complete.

To fix the problem you encountered, so we should collect up the promises from the async write and then await Promise.all(promises) at some point.

[andreialecu]

Even mechanical drives have caches, so this impact is mostly theoretical.

Here's a benchmark script:

const fs = require("fs/promises");
const { writeFileSync } = require("fs");
const path = require("path");

// Benchmark Helper Function
async function benchmark(fn, label) {
  const start = process.hrtime.bigint();
  await fn();
  const end = process.hrtime.bigint();
  console.log(`${label} took ${(end - start) / 1000000n} ms`);
}

// Generate 100KB of data
const dataSize = 100 * 1024; // 100KB
const data = "A".repeat(dataSize); // Repeat character 'A' to make up 100KB
const iterations = 1000; // How many times to write the file for benchmarking
const tempDir = "./temp"; // Directory to store test files

// Ensure temporary directory exists
async function ensureDirExists() {
  try {
    await fs.mkdir(tempDir, { recursive: true });
  } catch (err) {
    console.error("Error creating directory", err);
  }
}

// Async Write Function with Promise.all
async function writeFilesAsync() {
  const promises = [];
  for (let i = 0; i < iterations; i++) {
    const filePath = path.join(tempDir, `test-file-async-${i}.txt`);
    promises.push(fs.writeFile(filePath, data));
  }
  await Promise.all(promises);
}

// Sync Write Function
function writeFilesSync() {
  for (let i = 0; i < iterations; i++) {
    const filePath = path.join(tempDir, `test-file-sync-${i}.txt`);
    writeFileSync(filePath, data);
  }
}

// Warm-Up Function
async function warmUp(fn, label, runs = 2) {
  console.log(`Running ${runs} warm-up runs for ${label}...`);
  for (let i = 0; i < runs; i++) {
    await fn(); // Perform the warm-up run
  }
  console.log(`${label} warm-up completed.`);
}

// Run the benchmarks
(async () => {
  await ensureDirExists();

  // Warm-up and Benchmark for Async (Promise.all) Write
  await warmUp(writeFilesAsync, "Async (Promise.all)");
  await benchmark(writeFilesAsync, "Async (Promise.all) file write");

  // Warm-up and Benchmark for Sync Write
  await warmUp(writeFilesSync, "Sync");
  await benchmark(writeFilesSync, "Sync file write");
})();

Writing 1000 * 100KB files takes less than 100ms on my machine:

Running 2 warm-up runs for Async (Promise.all)...
Async (Promise.all) warm-up completed.
Async (Promise.all) file write took 73 ms
Running 2 warm-up runs for Sync...
Sync warm-up completed.
Sync file write took 102 ms

The async version is slightly faster, but it still takes less than 1 ms per write.

It takes 0.1 seconds to write 1000 images with writeFileSync.