Skip to content

FileOutput should write to FileOutputStream via BufferedOutputStream#2073

Merged
Godin merged 5 commits into
jacoco:masterfrom
Godin:BufferedFileOutput
Mar 23, 2026
Merged

FileOutput should write to FileOutputStream via BufferedOutputStream#2073
Godin merged 5 commits into
jacoco:masterfrom
Godin:BufferedFileOutput

Conversation

@Godin

@Godin Godin commented Mar 10, 2026

Copy link
Copy Markdown
Member

While working on #2065 I noticed that org.jacoco.agent.rt.internal.output.FileOutput does not write to FileOutputStream via BufferedOutputStream.

What seems to be regression after 4906467 of #61 in version 0.6.2

There is also inconsistency with org.jacoco.core.tools.ExecFileLoader which does.


Following test

	private static Thread newThread(final AgentOptions options,
			final RuntimeData runtimeData) {
		return new Thread() {
			@Override
			public void run() {
				FileOutput output = new FileOutput();
				try {
					output.startup(options, runtimeData);
					output.writeExecutionData(false);
				} catch (IOException e) {
					throw new RuntimeException(e);
				}
			}
		};
	}

	@Test
	public void test() throws Exception {
		File destFile = folder.newFile("jacoco.exec");
		AgentOptions options = new AgentOptions();
		options.setDestfile(destFile.getAbsolutePath());
		RuntimeData runtimeData = new RuntimeData();
		runtimeData.getExecutionData(1L, "Example", 1024 * 1024 * 8 * 4)
				.getProbes()[0] = true;
		Thread thread1 = newThread(options, runtimeData);
		Thread thread2 = newThread(options, runtimeData);
		thread1.start();
		thread2.start();
		thread1.join();
		thread2.join();
	}

demonstrates that one of the two JaCoCo runtimes writing to the same file within one JVM won't be able to wait for file lock release within allowed 3 seconds while other writes 4 MiB of execution data (33 554 432 probes)

Exception in thread "Thread-1" java.nio.channels.OverlappingFileLockException
	at sun.nio.ch.SharedFileLockTable.checkList(FileLockTable.java:255)
	at sun.nio.ch.SharedFileLockTable.add(FileLockTable.java:152)
	at sun.nio.ch.FileChannelImpl.lock(FileChannelImpl.java:1062)
	at java.nio.channels.FileChannel.lock(FileChannel.java:1053)
	at org.jacoco.agent.rt.internal.output.FileOutput.openFile(FileOutput.java:84)
	at org.jacoco.agent.rt.internal.output.FileOutput.startup(FileOutput.java:58)
	at org.jacoco.agent.rt.internal.output.FileOutputTest$1.run(FileOutputTest.java:154)

And while such situation seems unlikely - size of complete execution data for all classes in

  • org.jacoco.report/target/classes is about 3.8K (1 260 probes), org.jacoco.report.test produces about 9.8K
  • org.jacoco.core/target/classes is about 13K (3 773 probes), org.jacoco.core.test produces about 25K
  • jacoco-0.8.14.zip is about 98K (49 684 probes)
  • java.base.jmod of JDK 25 is about 394K (300 133 probes)
  • kotlin-compiler-2.3.0.zip is about 2.9M (1 335 895 probes)
  • wildfly-39.0.1.Final.zip is about 9M (4 469 140 probes)

IMO this is anyway good improvement.


For the record here is results of execution on my machine of benchmark added in this PR for org.jacoco.core.internal.data.CompactDataOutput:

Benchmark                                               (data)  (size)  Mode  Cnt    Score    Error  Units
CompactDataOutputBenchmark.toBufferedFileStream         STRING      32  avgt   20    0.301 ±  0.003  us/op
CompactDataOutputBenchmark.toBufferedFileStream         STRING     128  avgt   20    0.377 ±  0.003  us/op
CompactDataOutputBenchmark.toBufferedFileStream         STRING    1024  avgt   20    1.235 ±  0.025  us/op
CompactDataOutputBenchmark.toBufferedFileStream  BOOLEAN_ARRAY      32  avgt   20    0.289 ±  0.007  us/op
CompactDataOutputBenchmark.toBufferedFileStream  BOOLEAN_ARRAY     128  avgt   20    0.367 ±  0.003  us/op
CompactDataOutputBenchmark.toBufferedFileStream  BOOLEAN_ARRAY    1024  avgt   20    1.023 ±  0.038  us/op
CompactDataOutputBenchmark.toByteArrayStream            STRING      32  avgt   20    0.029 ±  0.001  us/op
CompactDataOutputBenchmark.toByteArrayStream            STRING     128  avgt   20    0.076 ±  0.003  us/op
CompactDataOutputBenchmark.toByteArrayStream            STRING    1024  avgt   20    0.522 ±  0.001  us/op
CompactDataOutputBenchmark.toByteArrayStream     BOOLEAN_ARRAY      32  avgt   20    0.026 ±  0.001  us/op
CompactDataOutputBenchmark.toByteArrayStream     BOOLEAN_ARRAY     128  avgt   20    0.099 ±  0.003  us/op
CompactDataOutputBenchmark.toByteArrayStream     BOOLEAN_ARRAY    1024  avgt   20    0.701 ±  0.030  us/op
CompactDataOutputBenchmark.toRawFileStream              STRING      32  avgt   20    1.328 ±  0.010  us/op
CompactDataOutputBenchmark.toRawFileStream              STRING     128  avgt   20    1.392 ±  0.026  us/op
CompactDataOutputBenchmark.toRawFileStream              STRING    1024  avgt   20    1.868 ±  0.023  us/op
CompactDataOutputBenchmark.toRawFileStream       BOOLEAN_ARRAY      32  avgt   20    6.769 ±  0.112  us/op
CompactDataOutputBenchmark.toRawFileStream       BOOLEAN_ARRAY     128  avgt   20   24.163 ±  1.496  us/op
CompactDataOutputBenchmark.toRawFileStream       BOOLEAN_ARRAY    1024  avgt   20  169.123 ±  1.836  us/op

@Godin Godin added this to the 0.8.15 milestone Mar 10, 2026
@Godin Godin self-assigned this Mar 10, 2026
@Godin Godin force-pushed the BufferedFileOutput branch from 5c23076 to 22625e7 Compare March 11, 2026 09:06
@Godin Godin force-pushed the BufferedFileOutput branch from 22625e7 to 723e54c Compare March 11, 2026 10:44
@Godin

Godin commented Mar 11, 2026

Copy link
Copy Markdown
Member Author

What seems to be regression after 4906467 of #61 in version 0.6.2

@marchof while this was 13 years ago, do you by chance remember 😅 or see any reasons why the use of BufferedOutputStream was removed, or was it simply overlooked? 😉 Also for the changelog entry I'm divided about how to classify this - as enhancement or bug-fix for regression? 😆

@Godin Godin requested a review from marchof March 13, 2026 16:48
@marchof

marchof commented Mar 15, 2026

Copy link
Copy Markdown
Member

@Godin Sure I clearly remember. I did this on purpose to see how long it takes you to find out about it. 🤣

Just joking: Many thanks for catching any analyzing this is detail!

@Godin Godin marked this pull request as ready for review March 23, 2026 13:13
@Godin Godin merged commit d91c6d8 into jacoco:master Mar 23, 2026
80 of 82 checks passed
@Godin Godin deleted the BufferedFileOutput branch March 23, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants