This is an automated email from the ASF dual-hosted git repository. tibordigana pushed a commit to branch SUREFIRE-1845 in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit 7b88ba501db85dbd56a99863b9f54f45bbdb359b Author: tibordigana <[email protected]> AuthorDate: Sat Sep 26 20:20:40 2020 +0200 [SUREFIRE-1845] Fixed the performance of Utf8RecodingDeferredFileOutputStream as a bottleneck for al tests with logs --- .../output/ThreadedStreamConsumer.java | 2 + .../surefire/report/StatelessXmlReporter.java | 1 - .../Utf8RecodingDeferredFileOutputStream.java | 151 +++++++++++++++++---- .../surefire/report/StatelessXmlReporterTest.java | 130 +++++++++++++++++- 4 files changed, 253 insertions(+), 31 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java index 1114948..10920c5 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java @@ -95,6 +95,8 @@ public final class ThreadedStreamConsumer } catch ( Throwable t ) { + // ensure the stack trace to be at the instance of the exception + t.getStackTrace(); errors.addException( t ); } } diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java index e1f56c2..7c24fe2 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java @@ -497,7 +497,6 @@ public class StatelessXmlReporter { xmlWriter.writeText( "" ); // Cheat sax to emit element outputStreamWriter.flush(); - utf8RecodingDeferredFileOutputStream.close(); eos.getUnderlying().write( ByteConstantsHolder.CDATA_START_BYTES ); // emit cdata utf8RecodingDeferredFileOutputStream.writeTo( eos ); eos.getUnderlying().write( ByteConstantsHolder.CDATA_END_BYTES ); diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java index fabf938..618e35e 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java @@ -19,12 +19,16 @@ package org.apache.maven.plugin.surefire.report; * under the License. */ -import org.apache.maven.surefire.shared.io.output.DeferredFileOutputStream; - import java.io.IOException; import java.io.OutputStream; +import java.io.RandomAccessFile; +import java.lang.ref.SoftReference; +import java.nio.ByteBuffer; +import java.nio.file.Files; +import java.nio.file.Path; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Objects.requireNonNull; import static org.apache.maven.surefire.api.util.internal.StringUtils.NL; /** @@ -35,14 +39,20 @@ import static org.apache.maven.surefire.api.util.internal.StringUtils.NL; */ final class Utf8RecodingDeferredFileOutputStream { - private final DeferredFileOutputStream deferredFileOutputStream; + private static final byte[] NL_BYTES = NL.getBytes( UTF_8 ); + public static final int CACHE_SIZE = 64 * 1024; + private final String channel; + private Path file; + private RandomAccessFile storage; private boolean closed; + private SoftReference<byte[]> largeCache; + private ByteBuffer cache; + private boolean isDirty; - @SuppressWarnings( "checkstyle:magicnumber" ) Utf8RecodingDeferredFileOutputStream( String channel ) { - deferredFileOutputStream = new DeferredFileOutputStream( 1_000_000, channel, "deferred", null ); + this.channel = requireNonNull( channel ); } public synchronized void write( String output, boolean newLine ) @@ -53,55 +63,142 @@ final class Utf8RecodingDeferredFileOutputStream return; } + if ( storage == null ) + { + file = Files.createTempFile( channel, "deferred" ); + storage = new RandomAccessFile( file.toFile(), "rw" ); + } + if ( output == null ) { output = "null"; } - deferredFileOutputStream.write( output.getBytes( UTF_8 ) ); - if ( newLine ) + if ( cache == null ) { - deferredFileOutputStream.write( NL.getBytes( UTF_8 ) ); + cache = ByteBuffer.allocate( CACHE_SIZE ); } - } - public long getByteCount() - { - return deferredFileOutputStream.getByteCount(); + isDirty = true; + + byte[] decodedString = output.getBytes( UTF_8 ); + int newLineLength = newLine ? NL_BYTES.length : 0; + if ( cache.remaining() >= decodedString.length + newLineLength ) + { + cache.put( decodedString ); + if ( newLine ) + { + cache.put( NL_BYTES ); + } + } + else + { + cache.flip(); + int minLength = cache.remaining() + decodedString.length + NL_BYTES.length; + byte[] buffer = getLargeCache( minLength ); + int bufferLength = 0; + System.arraycopy( cache.array(), cache.arrayOffset() + cache.position(), + buffer, bufferLength, cache.remaining() ); + bufferLength += cache.remaining(); + cache.clear(); + + System.arraycopy( decodedString, 0, buffer, bufferLength, decodedString.length ); + bufferLength += decodedString.length; + + if ( newLine ) + { + System.arraycopy( NL_BYTES, 0, buffer, bufferLength, NL_BYTES.length ); + bufferLength += NL_BYTES.length; + } + + storage.write( buffer, 0, bufferLength ); + } } - public synchronized void close() - throws IOException + public synchronized long getByteCount() { - closed = true; - deferredFileOutputStream.close(); + try + { + long length = 0; + if ( storage != null ) + { + sync(); + length = storage.length(); + } + return length; + } + catch ( IOException e ) + { + return 0; + } } + @SuppressWarnings( "checkstyle:innerassignment" ) public synchronized void writeTo( OutputStream out ) throws IOException { - if ( closed ) + if ( storage != null ) { - deferredFileOutputStream.writeTo( out ); + sync(); + storage.seek( 0L ); + byte[] buffer = new byte[CACHE_SIZE]; + for ( int readCount; ( readCount = storage.read( buffer ) ) != -1; ) + { + out.write ( buffer, 0, readCount ); + } } } public synchronized void free() { - if ( deferredFileOutputStream.getFile() != null ) + if ( !closed ) { - try + closed = true; + if ( cache != null ) { - close(); - if ( !deferredFileOutputStream.getFile().delete() ) + try { - deferredFileOutputStream.getFile().deleteOnExit(); + sync(); + storage.close(); + Files.delete( file ); + } + catch ( IOException e ) + { + file.toFile() + .deleteOnExit(); } } - catch ( IOException ioe ) - { - deferredFileOutputStream.getFile().deleteOnExit(); - } } } + + private void sync() throws IOException + { + if ( !isDirty ) + { + return; + } + + isDirty = false; + + cache.flip(); + byte[] array = cache.array(); + int offset = cache.arrayOffset() + cache.position(); + int length = cache.remaining(); + cache.clear(); + storage.write( array, offset, length ); + // the data that you wrote with the mode "rw" may still only be kept in memory and may be read back + // storage.getFD().sync(); + } + + @SuppressWarnings( "checkstyle:innerassignment" ) + private byte[] getLargeCache( int minLength ) + { + byte[] buffer; + if ( largeCache == null || ( buffer = largeCache.get() ) == null || buffer.length < minLength ) + { + buffer = new byte[minLength]; + largeCache = new SoftReference<>( buffer ); + } + return buffer; + } } diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java index 077a1a8..464c92b 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java @@ -21,17 +21,20 @@ package org.apache.maven.plugin.surefire.report; import junit.framework.TestCase; import org.apache.maven.plugin.surefire.booterclient.output.DeserializedStacktraceWriter; -import org.apache.maven.surefire.shared.utils.StringUtils; -import org.apache.maven.surefire.shared.utils.xml.Xpp3Dom; -import org.apache.maven.surefire.shared.utils.xml.Xpp3DomBuilder; import org.apache.maven.surefire.api.report.ReportEntry; import org.apache.maven.surefire.api.report.SimpleReportEntry; import org.apache.maven.surefire.api.report.StackTraceWriter; +import org.apache.maven.surefire.shared.utils.StringUtils; +import org.apache.maven.surefire.shared.utils.xml.Xpp3Dom; +import org.apache.maven.surefire.shared.utils.xml.Xpp3DomBuilder; import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.io.RandomAccessFile; +import java.nio.ByteBuffer; +import java.nio.file.Path; import java.util.Deque; import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap; @@ -39,6 +42,16 @@ import java.util.concurrent.atomic.AtomicInteger; import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.maven.surefire.api.util.internal.ObjectUtils.systemProps; +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.powermock.reflect.Whitebox.getInternalState; +import static org.powermock.reflect.Whitebox.invokeMethod; +import static org.powermock.reflect.Whitebox.setInternalState; +import static org.apache.maven.surefire.api.util.internal.StringUtils.NL; /** * @@ -275,6 +288,117 @@ public class StatelessXmlReporterTest assertNull( testCaseThree.getChild( "system-err" ) ); } + public void testNoWritesOnDeferredFile() throws Exception + { + Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" ); + out.free(); + out.write( "a", false ); + assertThat( (boolean) getInternalState( out, "isDirty" ) ) + .isFalse(); + } + + public void testLengthOnDeferredFile() throws Exception + { + Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" ); + + assertThat( out.getByteCount() ).isZero(); + + RandomAccessFile storage = mock( RandomAccessFile.class ); + setInternalState( out, "storage", storage ); + when( storage.length() ).thenReturn( 1L ); + assertThat( out.getByteCount() ).isEqualTo( 1 ); + + when( storage.length() ).thenThrow( IOException.class ); + assertThat( out.getByteCount() ).isZero(); + out.free(); + } + + @SuppressWarnings( "checkstyle:magicnumber" ) + public void testWritesOnDeferredFile() throws Exception + { + Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" ); + File file = new File( reportDir, "test" ); + setInternalState( out, "file", file.toPath() ); + for ( int i = 0; i < 33_000; i++ ) + { + out.write( "A", false ); + out.write( "B", true ); + } + out.write( null, false ); + out.write( null, true ); + assertThat( out.getByteCount() ) + .isEqualTo( 33_000 * ( 1 + 1 + NL.length() ) + 4 + 4 + NL.length() ); + out.free(); + } + + public void testFreeOnDeferredFile() throws Exception + { + Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" ); + setInternalState( out, "cache", ByteBuffer.allocate( 0 ) ); + Path path = mock( Path.class ); + File file = mock( File.class ); + when( path.toFile() ).thenReturn( file ); + setInternalState( out, "file", path ); + RandomAccessFile storage = mock( RandomAccessFile.class ); + doThrow( IOException.class ).when( storage ).close(); + setInternalState( out, "storage", storage ); + out.free(); + assertThat( (boolean) getInternalState( out, "closed" ) ).isTrue(); + verify( file, times( 1 ) ).deleteOnExit(); + } + + public void testCacheOnDeferredFile() throws Exception + { + Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" ); + byte[] b1 = invokeMethod( out, "getLargeCache", 1 ); + byte[] b2 = invokeMethod( out, "getLargeCache", 1 ); + assertThat( b1 ).isSameAs( b2 ); + assertThat( b1 ).hasSize( 1 ); + + byte[] b3 = invokeMethod( out, "getLargeCache", 2 ); + assertThat( b3 ).isNotSameAs( b1 ); + assertThat( b3 ).hasSize( 2 ); + + byte[] b4 = invokeMethod( out, "getLargeCache", 1 ); + assertThat( b4 ).isSameAs( b3 ); + assertThat( b3 ).hasSize( 2 ); + } + + public void testSyncOnDeferredFile() throws Exception + { + Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" ); + ByteBuffer cache = ByteBuffer.wrap( new byte[] {1, 2, 3} ); + cache.position( 3 ); + setInternalState( out, "cache", cache ); + assertThat( (boolean) getInternalState( out, "isDirty" ) ).isFalse(); + setInternalState( out, "isDirty", true ); + File file = new File( reportDir, "test" ); + setInternalState( out, "file", file.toPath() ); + RandomAccessFile storage = new RandomAccessFile( file, "rw" ); + setInternalState( out, "storage", storage ); + invokeMethod( out, "sync" ); + assertThat( (boolean) getInternalState( out, "isDirty" ) ).isFalse(); + storage.seek( 0L ); + assertThat( storage.read() ).isEqualTo( 1 ); + assertThat( storage.read() ).isEqualTo( 2 ); + assertThat( storage.read() ).isEqualTo( 3 ); + assertThat( storage.read() ).isEqualTo( -1 ); + assertThat( storage.length() ).isEqualTo( 3L ); + assertThat( cache.position() ).isEqualTo( 0 ); + assertThat( cache.limit() ).isEqualTo( 3 ); + storage.seek( 3L ); + invokeMethod( out, "sync" ); + assertThat( (boolean) getInternalState( out, "isDirty" ) ).isFalse(); + assertThat( storage.length() ).isEqualTo( 3L ); + assertThat( out.getByteCount() ).isEqualTo( 3L ); + assertThat( (boolean) getInternalState( out, "closed" ) ).isFalse(); + out.free(); + assertThat( (boolean) getInternalState( out, "closed" ) ).isTrue(); + assertThat( file ).doesNotExist(); + out.free(); + assertThat( (boolean) getInternalState( out, "closed" ) ).isTrue(); + } + private boolean defaultCharsetSupportsSpecialChar() { // some charsets are not able to deal with \u0115 on both ways of the conversion
