Skip to content

Commit

Permalink
fix: Reduced garbage on InsightsReport's serialization
Browse files Browse the repository at this point in the history
  • Loading branch information
franz1981 committed Jul 3, 2023
1 parent 94a97f6 commit 430ead9
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 66 deletions.
42 changes: 19 additions & 23 deletions api/src/main/java/com/redhat/insights/InsightsReport.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,10 @@
package com.redhat.insights;

import static com.redhat.insights.InsightsErrorCode.ERROR_SERIALIZING_TO_JSON;
import static com.redhat.insights.InsightsErrorCode.ERROR_WRITING_FILE;

import com.fasterxml.jackson.core.Version;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import java.io.IOException;
import java.io.StringWriter;
import java.io.*;
import java.util.Map;

/**
Expand Down Expand Up @@ -41,30 +37,30 @@ public interface InsightsReport {

void decorate(String key, String value);

/** Serializes this report to JSON on the given {@code file}. */
default void serialize(File out) {
// It uses RandomAccessFile on purpose vs Files::write
// to avoid any hidden pooled off-heap allocations
// at the cost of additional allocation/free and copy
try (RandomAccessFile raf = new RandomAccessFile(out, "rw")) {
// truncate it
raf.setLength(0);
ObjectMappers.createFor(this).writerWithDefaultPrettyPrinter().writeValue(raf, this);
} catch (IOException e) {
throw new InsightsException(ERROR_WRITING_FILE, "Could not serialize JSON to: " + out, e);
}
}

/**
* Serializes this report to JSON for transport
*
* @return JSON serialized report
* @return JSON serialized report as a stream of UTF-8 encoded bytes
*/
default String serialize() {
ObjectMapper mapper = new ObjectMapper();
mapper.registerModule(new JavaTimeModule());

SimpleModule simpleModule =
new SimpleModule(
"SimpleModule", new Version(1, 0, 0, null, "com.redhat.insights", "runtimes-java"));
simpleModule.addSerializer(InsightsReport.class, getSerializer());
for (InsightsSubreport subreport : getSubreports().values()) {
simpleModule.addSerializer(subreport.getClass(), subreport.getSerializer());
}
mapper.registerModule(simpleModule);

StringWriter writer = new StringWriter();
default byte[] serialize() {
try {
mapper.writerWithDefaultPrettyPrinter().writeValue(writer, this);
return ObjectMappers.createFor(this).writerWithDefaultPrettyPrinter().writeValueAsBytes(this);
} catch (IOException e) {
throw new InsightsException(ERROR_SERIALIZING_TO_JSON, "JSON serialization exception", e);
}
return writer.toString();
}
}
29 changes: 29 additions & 0 deletions api/src/main/java/com/redhat/insights/ObjectMappers.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* Copyright (C) Red Hat 2023 */
package com.redhat.insights;

import com.fasterxml.jackson.core.Version;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;

/**
* Utility class to provide a factory method for ObjectMapper used by {@link
* InsightsReport#serialize} methods
*/
class ObjectMappers {

public static ObjectMapper createFor(InsightsReport insightsReport) {
ObjectMapper mapper = new ObjectMapper();
mapper.registerModule(new JavaTimeModule());

SimpleModule simpleModule =
new SimpleModule(
"SimpleModule", new Version(1, 0, 0, null, "com.redhat.insights", "runtimes-java"));
simpleModule.addSerializer(InsightsReport.class, insightsReport.getSerializer());
for (InsightsSubreport subreport : insightsReport.getSubreports().values()) {
simpleModule.addSerializer(subreport.getClass(), subreport.getSerializer());
}
mapper.registerModule(simpleModule);
return mapper;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,15 @@
package com.redhat.insights.http;

import static com.redhat.insights.InsightsErrorCode.ERROR_UPLOAD_DIR_CREATION;
import static com.redhat.insights.InsightsErrorCode.ERROR_WRITING_FILE;

import com.redhat.insights.InsightsException;
import com.redhat.insights.InsightsReport;
import com.redhat.insights.config.InsightsConfiguration;
import com.redhat.insights.logging.InsightsLogger;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.io.*;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;

public class InsightsFileWritingClient implements InsightsHttpClient {
private final InsightsLogger logger;
Expand Down Expand Up @@ -46,21 +42,10 @@ public void decorate(InsightsReport report) {
@Override
public void sendInsightsReport(String filename, InsightsReport report) {
decorate(report);
String reportJson = report.serialize();
// logger.debug(reportJson);

// Can't reuse upload path - as this may be called as part of fallback
Path p = Paths.get(config.getArchiveUploadDir(), filename + ".json");
try {
Files.write(
p,
reportJson.getBytes(StandardCharsets.UTF_8),
StandardOpenOption.WRITE,
StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING);
} catch (IOException iox) {
throw new InsightsException(ERROR_WRITING_FILE, "Could not write to: " + p, iox);
}
report.serialize(p.toFile());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import com.redhat.insights.InsightsReport;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.zip.GZIPOutputStream;

/**
Expand Down Expand Up @@ -46,21 +45,19 @@ public interface InsightsHttpClient {
default boolean isReadyToSend() {
return true;
}

/**
* Static gzip helper method
*
* @param report
* @return gzipped bytes
*/
static byte[] gzipReport(final String report) {
try (final ByteArrayOutputStream baos = new ByteArrayOutputStream(report.length())) {
final byte[] buffy = report.getBytes(StandardCharsets.UTF_8);

static byte[] gzipReport(final byte[] report) {
try (final ByteArrayOutputStream baos = new ByteArrayOutputStream(report.length)) {
final GZIPOutputStream gzip = new GZIPOutputStream(baos);
gzip.write(buffy, 0, buffy.length);
gzip.write(report);
// An explicit close is necessary before we call toByteArray()
gzip.close();

return baos.toByteArray();
} catch (IOException iox) {
throw new InsightsException(ERROR_GZIP_FILE, "Failed to GZIP report: " + report, iox);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import com.redhat.insights.InsightsSubreport;
import com.redhat.insights.logging.InsightsLogger;
import java.io.IOException;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -49,7 +48,8 @@ public JsonSerializer<InsightsSubreport> getSerializer() {
return new JarInfoSubreportSerializer();
}

public String serializeReport() {
public byte[] serializeReport() {

ObjectMapper mapper = new ObjectMapper();

SimpleModule simpleModule =
Expand All @@ -58,12 +58,10 @@ public String serializeReport() {
simpleModule.addSerializer(getClass(), getSerializer());
mapper.registerModule(simpleModule);

StringWriter writer = new StringWriter();
try {
mapper.writerWithDefaultPrettyPrinter().writeValue(writer, this);
return mapper.writerWithDefaultPrettyPrinter().writeValueAsBytes(this);
} catch (IOException e) {
throw new InsightsException(ERROR_SERIALIZING_TO_JSON, "JSON serialization exception", e);
}
return writer.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void testJarFingerprinting() throws NoSuchAlgorithmException, IOException
report.generateReport(Filtering.DEFAULT);

// First, compute the SHA 512 fingerprint without the id hash
String initialReportJson = report.serialize();
byte[] initialReportJson = report.serialize();
final byte[] initialGz = gzipReport(initialReportJson);
final String hash = computeSha512(initialGz);

Expand All @@ -62,7 +62,7 @@ public void testJarFingerprinting() throws NoSuchAlgorithmException, IOException
assertEquals(controller.getIdHash(), hash);

// Reserialize with the hash
final String reportJson = report.serialize();
final byte[] reportJson = report.serialize();
final byte[] finalGz = gzipReport(reportJson);

assertNotEquals(initialReportJson, reportJson);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@
package com.redhat.insights.http;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import com.redhat.insights.InsightsException;
import com.redhat.insights.InsightsReport;
import com.redhat.insights.config.InsightsConfiguration;
import com.redhat.insights.doubles.DummyTopLevelReport;
import com.redhat.insights.doubles.NoopInsightsLogger;
import com.redhat.insights.logging.InsightsLogger;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import org.junit.jupiter.api.Test;

public class InsightsFileWritingClientTest {
Expand All @@ -37,14 +37,13 @@ public String getArchiveUploadDir() {

InsightsLogger logger = new NoopInsightsLogger();
InsightsHttpClient client = new InsightsFileWritingClient(logger, cfg);
InsightsReport report = mock(InsightsReport.class);
when(report.serialize()).thenReturn("foo");
InsightsReport report = new DummyTopLevelReport(logger, Collections.emptyMap());

client.sendInsightsReport("foo", report);
File[] files = tmpdir.toFile().listFiles();
assertEquals(1, files.length);
assertEquals("foo.json", files[0].getName());
assertEquals(3, files[0].length());
assertEquals(25, files[0].length());
// Cleanup
Files.delete(files[0].toPath());
Files.delete(tmpdir);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* Copyright (C) Red Hat 2023 */
package com.redhat.insights.http;

import static com.redhat.insights.InsightsErrorCode.ERROR_WRITING_FILE;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;

Expand All @@ -13,8 +14,10 @@
import com.redhat.insights.logging.InsightsLogger;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.Arrays;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -54,7 +57,24 @@ public String getArchiveUploadDir() {
InsightsLogger logger = new NoopInsightsLogger();

InsightsReport report = mock(InsightsReport.class);
when(report.serialize()).thenReturn("foo");
doAnswer(
invocationOnMock -> {
File out = invocationOnMock.getArgument(0, File.class);
Path p = out.toPath();
try {
Files.write(
out.toPath(),
"out".getBytes(StandardCharsets.UTF_8),
StandardOpenOption.WRITE,
StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING);
return null;
} catch (IOException iox) {
throw new InsightsException(ERROR_WRITING_FILE, "Could not write to: " + p, iox);
}
})
.when(report)
.serialize(any(File.class));

InsightsHttpClient failingClient = mock(InsightsHttpClient.class);
doThrow(new InsightsException("Failing on purpose"))
Expand Down Expand Up @@ -86,7 +106,7 @@ void theyAllFail() {
InsightsLogger logger = new NoopInsightsLogger();

InsightsReport report = mock(InsightsReport.class);
when(report.serialize()).thenReturn("foo");
when(report.serialize()).thenReturn("foo".getBytes(StandardCharsets.UTF_8));

InsightsHttpClient failingClient = mock(InsightsHttpClient.class);
doThrow(new InsightsException("Failing on purpose"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.redhat.insights.InsightsException;
import com.redhat.insights.InsightsReport;
Expand Down Expand Up @@ -287,7 +286,7 @@ private InsightsReport prepareReport() {
});
}

private Map<?, ?> parseReport(String report) throws JsonProcessingException {
private Map<?, ?> parseReport(byte[] report) throws IOException {
JsonMapper mapper = new JsonMapper();
return mapper.readValue(report, Map.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.redhat.insights.AbstractReportTest;
import java.nio.charset.StandardCharsets;
import java.util.*;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -41,7 +42,7 @@ public void testEmptyJarInfos() throws JsonProcessingException {
"Serializer of JarInfoSubreport should be JarInfoSubreportSerializer");

// generate report
String report = subreport.serializeReport();
String report = new String(subreport.serializeReport(), StandardCharsets.UTF_8);

// parse report and store it as a key-value map
Map<?, ?> map = parseReport(report);
Expand Down Expand Up @@ -85,7 +86,7 @@ public void testWithJarInfos() throws JsonProcessingException {
logger, Arrays.asList(jarInfoWithoutAttrs, jarInfoWithAttrs, JarInfo.MISSING));

// generate report
String report = subreport.serializeReport();
String report = new String(subreport.serializeReport(), StandardCharsets.UTF_8);
Map<?, ?> map = parseReport(report);

assertTrue(map.get("jars") instanceof List, "Jars in report should be parsed as list");
Expand All @@ -110,7 +111,7 @@ public void smokeTestClasspathJarsInReport() throws JsonProcessingException {
// generate report for jars in current classpath
classpathJarInfoSubreport.generateReport();

String report = classpathJarInfoSubreport.serializeReport();
String report = new String(classpathJarInfoSubreport.serializeReport(), StandardCharsets.UTF_8);
Map<?, ?> parsedReport = parseReport(report);
List<Map<String, ?>> jars = (List<Map<String, ?>>) parsedReport.get("jars");

Expand Down

0 comments on commit 430ead9

Please sign in to comment.