Faster File Provider Log Rotation

[juliusvaart]

rotateLogFileIfNeeded() runs before every single line and used to stat the log file each time to decide whether to rotate. write(...) then called synchronize() after every line. Both are syscalls, and both sit on an actor that every hot path in the extension awaits, so a burst of log lines became that many serialized disk operations in front of unrelated work. A bulk materialisation emits tens of thousands of lines: one measured pass produced 28,334 of them in seven minutes.

Track the bytes written instead of asking the file system. The counter is exact because this actor is the only writer and each file is created empty. Drop the per-line fsync: FileHandle.write(contentsOf:) is an unbuffered write(2), so the line is in the file and readable the moment it returns, and the fsync only bought durability against power loss, which a diagnostic log does not need. Rotation still flushes before closing a file.

Both substitutions are safe only because of properties that are easy to break later and invisible when broken, so each gets a test: the counter agrees with the file byte for byte, an unsynchronised line is readable as soon as the write returns, and rotation restarts the counter rather than carrying the closed file's total over.

Reaching those from a test needs somewhere to write, and a test bundle has no application group container. init therefore takes an optional logs directory and a maximum file size, mirroring FilesDatabaseManager(databaseDirectory:). Both default to today's behaviour.

Assisted-by: Claude Opus 5

Resolves

#

Summary

FileProviderLog is an actor, and every hot path in the File Provider extension awaits
it. Two syscalls per line sat on that actor:

• rotateLogFileIfNeeded() runs before every single line, and it stat-ed the log file
  each time to decide whether to rotate.
• write(...) called synchronize() after every line.

That is fine at a handful of lines a second and expensive in bulk. A materialisation pass
on a large synced folder emitted 28,334 lines in seven minutes, each one putting a stat
and an fsync in front of unrelated extension work.

This replaces the stat with a byte counter and removes the per-line fsync:

• The counter is exact because the actor is the only writer and each file is created
  empty.
• FileHandle.write(contentsOf:) is an unbuffered write(2), so a line is in the file
  and readable the moment the call returns. The fsync only added durability against power
  loss, which a diagnostic log does not need. Rotation still flushes before closing a file.

Net effect is two syscalls removed per line, one of them a disk flush.

Before / after

20,000 log lines written through the actor, three runs each, same machine. Measured by
building stable-34.0 with only this PR's init parameters applied (so both sides can be
driven from a test bundle) and running the identical probe against both:

	seconds for 20,000 lines	lines/s
stable-34.0	1.431 / 1.380 / 1.393	~14,400
this branch	0.199 / 0.198 / 0.206	~100,000

7.0x, median 1.393 s -> 0.199 s. Against the 28,334-line pass above that is roughly
1.97 s of actor time down to 0.28 s.

To reproduce, drop this into Tests/NextcloudFileProviderKitTests/ on either side and run
swift test --filter ThroughputProbe:

ThroughputProbe.swift

@preconcurrency import FileProvider
import Foundation
@testable import NextcloudFileProviderKit
import XCTest

final class ThroughputProbe: XCTestCase {
    func testLogWriteThroughput() async throws {
        let dir = FileManager.default.temporaryDirectory
            .appendingPathComponent("throughput-\(UUID().uuidString)", isDirectory: true)
        defer { try? FileManager.default.removeItem(at: dir) }

        let log = FileProviderLog(
            fileProviderDomainIdentifier: NSFileProviderDomainIdentifier("probe"),
            logsDirectory: dir,
            maxLogFileSize: 1024 * 1024 * 1024
        )

        let lines = 20_000
        let clock = ContinuousClock()
        let start = clock.now

        for index in 0 ..< lines {
            await log.write(
                category: "Probe", level: .info, message: "materialised item \(index)",
                details: [:], file: #file, function: #function, line: #line
            )
        }

        let elapsed = clock.now - start
        let seconds = Double(elapsed.components.seconds)
            + Double(elapsed.components.attoseconds) * 1e-18
        print("lines_per_s=\(Double(lines) / seconds)")
    }
}

The probe itself is deliberately not committed. A wall-clock assertion is flaky on
hosted runners, so the tests that ship assert correctness properties instead.

Tests
Both substitutions are safe only because of properties that are easy to break later and
invisible when they break, so each gets a guard in FileProviderLogWriteTests:

the byte counter agrees with the file on disk exactly — drift compounds silently, too high and the log rotates early forever, too low and it never rotates
a line is readable as soon as write returns — the sole justification for dropping the fsync; if it stops holding, the log loses its newest lines, which are the ones a crash investigation wants
rotation restarts the counter rather than carrying the closed file's total over, which would leave it above the limit permanently and rotate on every subsequent line
Note on the API change
FileProviderLog.init gains an optional logs directory and maximum file size. A test
bundle has no application group container, so without a directory to write to none of the
above is reachable from a test. This mirrors FilesDatabaseManager(databaseDirectory:),
which exists for the same reason. Both parameters default to current behaviour.

Checklist

• Sign-off message is added to all commits.
• The commit messages are following The Conventional Commits specification.
• The commit history is clean with no merge commits.
  • How to rebase your commits
  • How to squash commits with rebase
• Uploaded screenshots from before and after for UI changes.
• Test(s) added to branch, with before/after results included in PR description, for performance improvements where applicable.
• Documentation has been updated or is not required.
• Backports requested where applicable (critical bugfixes).
• Labels added where applicable (bug/enhancement).
• Milestone added for target version.

AI (if applicable)

• The content of this PR was partly or fully generated using AI
  • I have read the guidelines for AI-assisted contributions.
  • I have read Nextcloud's AI Contribution Policy.

[i2h3]

While I personally consider the practical performance impact neglectable (I see the substantial bottlenecks elsewhere), this optimization still appears reasonable and justified to scale better.

I will definitely remember this pattern in the future. It makes much more sense than the repeated calls.

[i2h3]

@juliusvaart Oh, oops… I just noticed: you opened the pull request to be merged into stable-34.0. 😬 Please always open pull requests against master! We need it in that development branch and then can do our ports back to release branches like stable-34.0. Right now, this would ship with the next 34 release but not end up in any future major release.

[i2h3]

/backport to master

[i2h3]

Uh… I just tried whether our backport bot can actually do the reverse of what it usually does… Maybe, maybe, maybe… Otherwise: We need another PR to master then. I requested an update on the unreviewed PRs to be changed to be against master.

[juliusvaart]

Now based on master

[i2h3]

Same as in #10734: the failed DCO check can be ignored. When the other checks succeed, we can merge.

[claucambra]

Just some complaints about claudeisms (bad commenting practices)

[github-actions]

Artifact containing the AppImage: nextcloud-appimage-pr-10741.zip

Digest: sha256:4515b1dc68bc54a18d462551cd5dae7886cd31a625ec82709e59232b55dd7388

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

[backportbot]

The backport to master failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout master
git pull origin master

# Create the new backport branch
git checkout -b backport/10741/master

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 770a16e1

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/10741/master

Error: Failed to check for changes with origin/master: No changes found in backport branch

---

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

[i2h3]

/backport to stable-34.0