Perf: Remove unnecessary fsync from file I/O (6x speedup)#309
Merged
Conversation
JFR profiling revealed that 94% of native method time in I/O-heavy workloads (like Image::ExifTool) was spent in FileChannel.force() calls - synchronous disk flushes (fsync). The previous implementation incorrectly called force(true) on every flush() and close(), which is extremely slow (10-100ms per call). Changes: - CustomFileChannel.close(): Remove force() - OS flushes on close - CustomFileChannel.flush(): Now no-op (FileChannel is unbuffered) - CustomFileChannel.sync(): NEW - explicit fsync when truly needed - CustomFileChannel.fileno(): Return synthetic fd (was undef) - IOHandle interface: Add sync() with documentation on flush vs sync - LayeredIOHandle: Delegate sync() to underlying handle - IOHandle perlmodule: Wire up _sync() to call ioHandle.sync() - GlobalContext: Initialize IOHandle methods at startup - IO/Handle.pm: Fix Java backend detection Perl semantics preserved: - flush() flushes app buffers to OS kernel buffer - sync() calls fsync to write to physical disk (IO::Handle->sync) Performance: - ExifTool.t: 16.7s -> 2.9s (5.8x faster) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
The synthetic fileno implementation broke tests that relied on undef == undef comparison (io/perlio_leaks.t) and consistent fd numbers. Changes: - CustomFileChannel.fileno(): Return undef (matches master behavior) - IO/Handle.pm sync()/flush(): Remove `defined fileno()` checks since Java backend handles invalid handles internally Test results restored to master baseline: - io/perlio_leaks.t: 12/12 (was 0/12) - io/dup.t: 26/29 (was 17/29) - op/require_37033.t: 7/10 (unchanged) Performance still improved: ExifTool.t 3.2s (was 16.7s) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JFR profiling revealed that 94% of native method time in I/O-heavy workloads (like Image::ExifTool) was spent in
FileChannel.force()calls - synchronous disk flushes (fsync).The previous implementation incorrectly called
force(true)on everyflush()andclose(), which is extremely slow (10-100ms per call).Changes
Core fix (
CustomFileChannel.java)close(): Removeforce()- OS flushes kernel buffers on close anywayflush(): Now a no-op (FileChannel is unbuffered, writes go directly to OS)sync(): NEW - explicit fsync when truly needed (maps toIO::Handle->sync)fileno(): Return synthetic fd (was undef, which brokedefined fileno($fh)checks)Interface updates
IOHandle.java(interface): Added comprehensive Javadoc explaining flush vs sync semantics; added defaultsync()methodLayeredIOHandle.java: Delegatesync()to underlying handlePerl bindings
IOHandle.java(perlmodule): Wire up_sync()to callioHandle.sync()GlobalContext.java: Initialize IOHandle methods at startupIO/Handle.pm: Fix Java backend detection ($has_java_backend)Perl Semantics Preserved
flush()fflush())sync()IO::Handle->sync)Performance
Test Plan
./gradlew test)print "prompt: "; $v = <>)IO::Handle->syncreturns "0 but true" per Perl conventionGenerated with Devin