Apache
/
maven
mirror of https://github.com/apache/maven.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[MNG-7899] Various memory usage improvements 4 (#1269) 

- Use the main StringBuilder to append string instead of using a separate one
- Use the StringBuilder.append() with index to avoid String.substring(), less temporary strings
- Reuse the FileSizeFormat object in the while loop avoiding multiple temporary instances creation
- Non-threadsafe FileSizeFormat instance can be make class instance since its formatProgress() method is only called in a synchronized block.
- add a test in a multi-threaded context
- Non-threadsafe StringBuilder instance 'buffer' can be make class instance since it is always called in synchronized methods
- remove synchronized block in transferProgressed() method, the method is synchronized and the block is not needed
- Remove the Collections.synchronizedMap since all methods that use the transfers map are synchronized
This commit is contained in:
sebastien-doyon 2023-10-20 14:16:34 +02:00 committed by GitHub
parent 9abaf3aa1c
commit 68bbc8f393
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 160 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java
View File

@ -19,7 +19,6 @@
package org.apache.maven.cli.transfer;
import java.io.PrintStream;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Locale;
@ -35,7 +34,9 @@ import org.eclipse.aether.transfer.TransferResource;
*/
public class ConsoleMavenTransferListener extends AbstractMavenTransferListener {
private Map<TransferResource, Long> transfers = Collections.synchronizedMap(new LinkedHashMap<>());
private Map<TransferResource, Long> transfers = new LinkedHashMap<>();
private FileSizeFormat format = new FileSizeFormat(Locale.ENGLISH); // use in a synchronized fashion
private StringBuilder buffer = new StringBuilder(128); // use in a synchronized fashion
private boolean printResourceNames;
private int lastLength;
@ -64,22 +65,38 @@ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener
TransferResource resource = event.getResource();
transfers.put(resource, event.getTransferredBytes());
StringBuilder buffer = new StringBuilder(128);
buffer.append("Progress (").append(transfers.size()).append("): ");
synchronized (transfers) {
Iterator<Map.Entry<TransferResource, Long>> entries =
transfers.entrySet().iterator();
while (entries.hasNext()) {
Map.Entry<TransferResource, Long> entry = entries.next();
long total = entry.getKey().getContentLength();
Long complete = entry.getValue();
buffer.append(getStatus(entry.getKey().getResourceName(), complete, total));
String resourceName = entry.getKey().getResourceName();
if (printResourceNames) {
int idx = resourceName.lastIndexOf('/');
if (idx < 0) {
buffer.append(resourceName);
} else {
buffer.append(resourceName, idx + 1, resourceName.length());
}
buffer.append(" (");
}
buffer.append(format.formatProgress(complete, total));
if (printResourceNames) {
buffer.append(")");
}
if (entries.hasNext()) {
buffer.append(" | ");
}
}
}
int pad = lastLength - buffer.length();
lastLength = buffer.length();
@ -87,25 +104,7 @@ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener
buffer.append('\r');
out.print(buffer);
out.flush();
}
private String getStatus(String resourceName, long complete, long total) {
FileSizeFormat format = new FileSizeFormat(Locale.ENGLISH);
StringBuilder status = new StringBuilder();
if (printResourceNames) {
int idx = resourceName.lastIndexOf('/');
status.append(idx < 0 ? resourceName : resourceName.substring(idx + 1));
status.append(" (");
}
status.append(format.formatProgress(complete, total));
if (printResourceNames) {
status.append(")");
}
return status.toString();
buffer.setLength(0);
}
private void pad(StringBuilder buffer, int spaces) {
@ -135,12 +134,12 @@ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener
private void overridePreviousTransfer(TransferEvent event) {
if (lastLength > 0) {
StringBuilder buffer = new StringBuilder(128);
pad(buffer, lastLength);
buffer.append('\r');
out.print(buffer);
out.flush();
lastLength = 0;
buffer.setLength(0);
}
}
}

128
maven-embedder/src/test/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListenerTest.java Normal file
View File

@ -0,0 +1,128 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.cli.transfer;
import java.io.FileNotFoundException;
import java.io.PrintStream;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import org.eclipse.aether.DefaultRepositorySystemSession;
import org.eclipse.aether.transfer.TransferCancelledException;
import org.eclipse.aether.transfer.TransferEvent;
import org.eclipse.aether.transfer.TransferResource;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertTrue;
class ConsoleMavenTransferListenerTest {
private CountDownLatch startLatch;
private CountDownLatch endLatch;
@Test
void testTransferProgressedWithPrintResourceNames() throws FileNotFoundException, InterruptedException {
int size = 1000;
ExecutorService service = Executors.newFixedThreadPool(size * 2);
startLatch = new CountDownLatch(size);
endLatch = new CountDownLatch(size);
Map<String, String> output = new ConcurrentHashMap<String, String>();
ConsoleMavenTransferListener listener = new ConsoleMavenTransferListener(
new PrintStream(System.out) {
@Override
public void print(Object o) {
String string = o.toString();
int i = string.length() - 1;
while (i >= 0) {
char c = string.charAt(i);
if (c == '\n' || c == '\r' || c == ' ') i--;
else break;
}
string = string.substring(0, i + 1).trim();
output.put(string, string);
System.out.print(o);
}
},
true);
TransferResource resource = new TransferResource(null, null, "http://maven.org/test/test-resource", null, null);
resource.setContentLength(size - 1);
DefaultRepositorySystemSession session = new DefaultRepositorySystemSession();
// warm up
test(listener, session, resource, 0);
for (int i = 1; i < size; i++) {
final int bytes = i;
service.execute(() -> {
test(listener, session, resource, bytes);
});
}
// start all threads at once
try {
startLatch.await();
} catch (InterruptedException e) {
e.printStackTrace();
}
// wait for all thread to end
try {
endLatch.await();
} catch (InterruptedException e) {
e.printStackTrace();
}
StringBuilder message = new StringBuilder("Messages [");
boolean test = true;
for (int i = 0; i < 999; i++) {
boolean ok = output.containsKey("Progress (1): test-resource (" + i + "/999 B)");
if (!ok) {
System.out.println("false : " + i);
message.append(i + ",");
}
test = test & ok;
}
assertTrue(test, message.toString() + "] are missiong in " + output);
}
private void test(
ConsoleMavenTransferListener listener,
DefaultRepositorySystemSession session,
TransferResource resource,
final int bytes) {
TransferEvent event = new TransferEvent.Builder(session, resource)
.setTransferredBytes(bytes)
.build();
startLatch.countDown();
try {
listener.transferProgressed(event);
} catch (TransferCancelledException e) {
}
endLatch.countDown();
}
}