This is an automated email from the ASF dual-hosted git repository.
tibordigana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
The following commit(s) were added to refs/heads/master by this push:
new 32bd56b [SUREFIRE-1845] Fixed the performance of
Utf8RecodingDeferredFileOutputStream as a bottleneck for the tests with logs
32bd56b is described below
commit 32bd56b4ea908147592ef92c71c4e7936e070993
Author: tibordigana <[email protected]>
AuthorDate: Sat Sep 26 20:20:40 2020 +0200
[SUREFIRE-1845] Fixed the performance of
Utf8RecodingDeferredFileOutputStream as a bottleneck for the tests with logs
---
.../output/ThreadedStreamConsumer.java | 2 +
.../surefire/report/StatelessXmlReporter.java | 1 -
.../Utf8RecodingDeferredFileOutputStream.java | 151 +++++++++++++++++----
.../surefire/report/StatelessXmlReporterTest.java | 142 ++++++++++++++++++-
4 files changed, 265 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..188e201 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,21 @@ 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.ByteArrayOutputStream;
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 +43,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 +289,128 @@ 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" );
+ 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()
);
+
+ StringBuilder expectedContent = new StringBuilder( 150_000 );
+ for ( int i = 0; i < 33_000; i++ )
+ {
+ expectedContent.append( 'A' ).append( 'B' ).append( NL );
+ }
+ expectedContent.append( "null" ).append( "null" ).append( NL );
+ ByteArrayOutputStream read = new ByteArrayOutputStream( 150_000 );
+ out.writeTo( read );
+ assertThat( read.toString() )
+ .isEqualTo( expectedContent.toString() );
+
+ 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
