olaf-otto commented on a change in pull request #50: WAGON-537 Maven download speed of large artifacts is slow URL: https://github.com/apache/maven-wagon/pull/50#discussion_r228322159
########## File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/AbstractWagon.java ########## @@ -560,31 +564,78 @@ protected void transfer( Resource resource, InputStream input, OutputStream outp protected void transfer( Resource resource, InputStream input, OutputStream output, int requestType, long maxSize ) throws IOException { - byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; + byte[] buffer = bufferForTransferring( resource ); TransferEvent transferEvent = new TransferEvent( this, resource, TransferEvent.TRANSFER_PROGRESS, requestType ); transferEvent.setTimestamp( System.currentTimeMillis() ); long remaining = maxSize; while ( remaining > 0 ) { - // let's safely cast to int because the min value will be lower than the buffer size. - int n = input.read( buffer, 0, (int) Math.min( buffer.length, remaining ) ); + // Read from the stream, block if necessary until either EOF or buffer is filled. + // Filling the buffer has priority since downstream processors will significantly degrade i/o + // performance if called to frequently (large data streams) as they perform expensive tasks such as + // console output or data integrity checks. + int nextByte = input.read(); - if ( n == -1 ) + if ( nextByte == -1 ) { break; } - fireTransferProgress( transferEvent, buffer, n ); + buffer[0] = ( byte ) nextByte; + + // let's safely cast to int because the min value will be lower than the buffer size. + int length = (int) min( buffer.length, remaining ), + read = 1; + + for ( ; read < length ; ++read ) + { + nextByte = input.read(); + if ( nextByte == -1 ) + { + break; + } + buffer[read] = ( byte ) nextByte; + } + + fireTransferProgress( transferEvent, buffer, read ); Review comment: Hi @michael-o I am going to retract htis pull request to change as I am not satisfied with the solution yet. As it happens, the same issue exist when uploading large files (100+ MB) using maven. Thee, however, buffering has a different effect as the underlying stream usually stems from a file. I have decided that I will address both issues. The best course of action is thus to change the scope of WAGON-537 and adopt a more generic solution (using nio) that will fix the issue for both cases. I will open up a ne wpoull request asap. Thanks again for your review! ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services