From 3fa5b8b4e3c67a4e81961ce4ee9a7a3b1aa8ab24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20G=C3=B3ral?= <60390247+jan-gogo@users.noreply.github.com> Date: Thu, 7 May 2020 19:55:33 +0200 Subject: [PATCH] Reduce memory usage by using Reader and Writer instead of ByteArrays (#767) * Reduce memory usage by using Reader and Writer instead of ByteArrays where possible. --- release_notes.md | 1 + .../src/main/kotlin/ftl/args/AndroidArgs.kt | 13 +++++--- .../src/main/kotlin/ftl/args/IosArgs.kt | 12 ++++--- .../kotlin/ftl/args/yml/YamlDeprecated.kt | 12 ++++--- .../src/main/kotlin/ftl/doctor/Doctor.kt | 7 ++-- .../src/main/kotlin/ftl/mock/MockServer.kt | 5 ++- .../main/kotlin/ftl/reports/xml/JUnitXml.kt | 32 ++++++------------- test_runner/src/main/kotlin/ftl/util/Utils.kt | 6 ++-- test_runner/src/test/kotlin/Debug.kt | 4 +-- .../test/kotlin/ftl/args/AndroidArgsTest.kt | 3 ++ .../src/test/kotlin/ftl/args/IosArgsTest.kt | 3 ++ .../kotlin/ftl/args/yml/YamlDeprecatedTest.kt | 3 ++ .../src/test/kotlin/ftl/doctor/DoctorTest.kt | 4 +++ 13 files changed, 60 insertions(+), 45 deletions(-) diff --git a/release_notes.md b/release_notes.md index 6cd6f3c13f..85d9aa6eca 100644 --- a/release_notes.md +++ b/release_notes.md @@ -1,4 +1,5 @@ ## next (unreleased) +- [#757](https://github.com/Flank/flank/pull/767) Reduce memory usage by using Reader and Writer instead of ByteArrays. ([jan-gogo](https://github.com/jan-gogo)) - [#763](https://github.com/Flank/flank/pull/763) Use "localhost" as default for hostname to fix backward compatibility. ([jan-gogo](https://github.com/jan-gogo)) - [#757](https://github.com/Flank/flank/pull/757) Print version and revision before each command. ([jan-gogo](https://github.com/jan-gogo)) - [#759](https://github.com/Flank/flank/pull/759) Add shard name for uploaded xctestrun files. ([pawelpasterz](https://github.com/pawelpasterz)) diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt index dfa107c9ec..dfb93ddb1a 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt @@ -1,5 +1,6 @@ package ftl.args +import com.google.common.annotations.VisibleForTesting import ftl.android.AndroidCatalog import ftl.android.IncompatibleModelVersion import ftl.android.SupportedDeviceConfig @@ -29,6 +30,7 @@ import ftl.config.Device import ftl.config.FtlConstants import ftl.config.parseRoboDirectives import ftl.util.FlankFatalError +import java.io.Reader import java.nio.file.Files import java.nio.file.Path @@ -187,11 +189,11 @@ AndroidArgs mergeYmlMaps(GcloudYml, AndroidGcloudYml, FlankYml, AndroidFlankYml) } - fun load(data: Path, cli: AndroidRunCommand? = null): AndroidArgs = - load(String(Files.readAllBytes(data)), cli) + fun load(yamlPath: Path, cli: AndroidRunCommand? = null): AndroidArgs = load(Files.newBufferedReader(yamlPath), cli) - fun load(yamlData: String, cli: AndroidRunCommand? = null): AndroidArgs { - val data = YamlDeprecated.modifyAndThrow(yamlData, android = true) + @VisibleForTesting + internal fun load(yamlReader: Reader, cli: AndroidRunCommand? = null): AndroidArgs { + val data = YamlDeprecated.modifyAndThrow(yamlReader, android = true) val flankYml = yamlMapper.readValue(data, FlankYml::class.java) val gcloudYml = yamlMapper.readValue(data, GcloudYml::class.java) @@ -215,7 +217,8 @@ AndroidArgs FlankYml(), AndroidFlankYml(), "", - AndroidRunCommand()) + AndroidRunCommand() + ) } } } diff --git a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt index 0107434f80..b9b5f6af4e 100644 --- a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt +++ b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt @@ -1,5 +1,6 @@ package ftl.args +import com.google.common.annotations.VisibleForTesting import ftl.args.ArgsHelper.assertCommonProps import ftl.args.ArgsHelper.assertFileExists import ftl.args.ArgsHelper.assertGcsFileExists @@ -24,6 +25,7 @@ import ftl.ios.IosCatalog import ftl.ios.Xctestrun import ftl.util.FlankTestMethod import ftl.util.FlankFatalError +import java.io.Reader import java.nio.file.Files import java.nio.file.Path @@ -156,10 +158,11 @@ IosArgs mergeYmlMaps(GcloudYml, IosGcloudYml, FlankYml, IosFlankYml) } - fun load(data: Path, cli: IosRunCommand? = null): IosArgs = load(String(Files.readAllBytes(data)), cli) + fun load(yamlPath: Path, cli: IosRunCommand? = null): IosArgs = load(Files.newBufferedReader(yamlPath), cli) - fun load(yamlData: String, cli: IosRunCommand? = null): IosArgs { - val data = YamlDeprecated.modifyAndThrow(yamlData, android = false) + @VisibleForTesting + internal fun load(yamlReader: Reader, cli: IosRunCommand? = null): IosArgs { + val data = YamlDeprecated.modifyAndThrow(yamlReader, android = false) val flankYml = yamlMapper.readValue(data, FlankYml::class.java) val iosFlankYml = yamlMapper.readValue(data, IosFlankYml::class.java) @@ -183,7 +186,8 @@ IosArgs FlankYml(), IosFlankYml(), "", - IosRunCommand()) + IosRunCommand() + ) } } } diff --git a/test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt b/test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt index efdf927409..9ca2944a3b 100644 --- a/test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt +++ b/test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt @@ -4,8 +4,10 @@ import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.databind.node.JsonNodeFactory import com.fasterxml.jackson.databind.node.MissingNode import com.fasterxml.jackson.databind.node.ObjectNode +import com.google.common.annotations.VisibleForTesting import ftl.args.ArgsHelper.yamlMapper import ftl.util.FlankFatalError +import java.io.Reader import java.nio.file.Files import java.nio.file.Path @@ -123,8 +125,7 @@ object YamlDeprecated { fun modify(yamlPath: Path): Boolean { if (yamlPath.toFile().exists().not()) throw FlankFatalError("Flank yml doesn't exist at path $yamlPath") - val data = String(Files.readAllBytes(yamlPath)) - val (errorDetected, string) = modify(data) + val (errorDetected, string) = modify(Files.newBufferedReader(yamlPath)) Files.write(yamlPath, string.toByteArray()) println("\nUpdated ${yamlPath.fileName} file") @@ -133,8 +134,8 @@ object YamlDeprecated { } // Throw exception when Level.Error modified key is found. - fun modifyAndThrow(yamlData: String, android: Boolean): String { - val (error, data) = YamlDeprecated.modify(yamlData) + fun modifyAndThrow(yamlReader: Reader, android: Boolean): String { + val (error, data) = modify(yamlReader) if (error) { val platform = if (android) "android" else "ios" @@ -144,7 +145,8 @@ object YamlDeprecated { return data } - fun modify(yamlData: String): Pair { + @VisibleForTesting + internal fun modify(yamlData: Reader): Pair { val mappedYaml = yamlMapper.readTree(yamlData) val parsed = if (mappedYaml == null || mappedYaml is MissingNode) { diff --git a/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt b/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt index f2e279eede..be21ff87a3 100644 --- a/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt +++ b/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt @@ -1,17 +1,20 @@ package ftl.doctor +import com.google.common.annotations.VisibleForTesting import ftl.args.ArgsHelper import ftl.args.IArgsCompanion +import java.io.Reader import java.nio.file.Files import java.nio.file.Path object Doctor { fun validateYaml(args: IArgsCompanion, data: Path): String { if (!data.toFile().exists()) return "Skipping yaml validation. No file at path $data" - return validateYaml(args, String(Files.readAllBytes(data))) + return validateYaml(args, Files.newBufferedReader(data)) } - fun validateYaml(args: IArgsCompanion, data: String): String { + @VisibleForTesting + internal fun validateYaml(args: IArgsCompanion, data: Reader): String { var result = "" val parsed = ArgsHelper.yamlMapper.readTree(data) diff --git a/test_runner/src/main/kotlin/ftl/mock/MockServer.kt b/test_runner/src/main/kotlin/ftl/mock/MockServer.kt index 99316348b3..b0ed281876 100644 --- a/test_runner/src/main/kotlin/ftl/mock/MockServer.kt +++ b/test_runner/src/main/kotlin/ftl/mock/MockServer.kt @@ -57,9 +57,8 @@ object MockServer { private inline fun loadCatalog(fileName: String): T { val jsonPath = Paths.get("./src/test/kotlin/ftl/fixtures/$fileName") - if (!jsonPath.toFile().exists()) throw RuntimeException("Path doesn't exist: $fileName") - val jsonString = String(Files.readAllBytes(jsonPath)) - return JSON_FACTORY.fromString(jsonString, T::class.java) + if (!Files.exists(jsonPath)) throw RuntimeException("Path doesn't exist: $fileName") + return JSON_FACTORY.fromReader(Files.newBufferedReader(jsonPath), T::class.java) } private val androidCatalog by lazy { loadCatalog("android_catalog.json") } diff --git a/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt b/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt index cda3835a7f..b94676aa43 100644 --- a/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt +++ b/test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt @@ -7,7 +7,6 @@ import com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PRO import ftl.reports.xml.model.JUnitTestResult import ftl.reports.xml.model.JUnitTestSuite import java.io.File -import java.nio.file.Files import java.nio.file.Path private val xmlModule = JacksonXmlModule().apply { setDefaultUseWrapper(false) } @@ -17,47 +16,36 @@ private val xmlMapper = XmlMapper(xmlModule) internal val xmlPrettyWriter = xmlMapper.writerWithDefaultPrettyPrinter() -private fun xmlBytes(path: Path): ByteArray { - if (!path.toFile().exists()) throw RuntimeException("$path doesn't exist!") - return Files.readAllBytes(path) -} - fun JUnitTestResult?.xmlToString(): String { if (this == null) return "" val prefix = "\n" return prefix + xmlPrettyWriter.writeValueAsString(this) } -fun parseOneSuiteXml(bytes: ByteArray): JUnitTestResult { - return JUnitTestResult(mutableListOf(xmlMapper.readValue(bytes, JUnitTestSuite::class.java))) -} - fun parseOneSuiteXml(path: Path): JUnitTestResult { - return parseOneSuiteXml(xmlBytes(path)) + return parseOneSuiteXml(path.toFile()) } -fun parseOneSuiteXml(path: File): JUnitTestResult { - return parseOneSuiteXml(xmlBytes(path.toPath())) +fun parseOneSuiteXml(file: File): JUnitTestResult { + if (!file.exists()) throw RuntimeException("$file doesn't exist!") + return JUnitTestResult(mutableListOf(xmlMapper.readValue(file, JUnitTestSuite::class.java))) } fun parseOneSuiteXml(data: String): JUnitTestResult { - return parseOneSuiteXml(data.toByteArray()) + return JUnitTestResult(mutableListOf(xmlMapper.readValue(data, JUnitTestSuite::class.java))) } // -- -fun parseAllSuitesXml(bytes: ByteArray): JUnitTestResult { - return xmlMapper.readValue(bytes, JUnitTestResult::class.java) -} - fun parseAllSuitesXml(path: Path): JUnitTestResult { - return parseAllSuitesXml(xmlBytes(path)) + return parseAllSuitesXml(path.toFile()) } -fun parseAllSuitesXml(path: File): JUnitTestResult { - return parseAllSuitesXml(path.toPath()) +fun parseAllSuitesXml(file: File): JUnitTestResult { + if (!file.exists()) throw RuntimeException("$file doesn't exist!") + return xmlMapper.readValue(file, JUnitTestResult::class.java) } fun parseAllSuitesXml(data: String): JUnitTestResult { - return parseAllSuitesXml(data.toByteArray()) + return xmlMapper.readValue(data, JUnitTestResult::class.java) } diff --git a/test_runner/src/main/kotlin/ftl/util/Utils.kt b/test_runner/src/main/kotlin/ftl/util/Utils.kt index e6915eeca4..c607c400c7 100644 --- a/test_runner/src/main/kotlin/ftl/util/Utils.kt +++ b/test_runner/src/main/kotlin/ftl/util/Utils.kt @@ -120,8 +120,10 @@ fun copyBinaryResource(name: String) { destinationPath.parent.toFile().mkdirs() // "binaries/" folder prefix is required for Linux to find the resource. - val bytes = getResource("binaries/$name").use { it.readBytes() } - Files.write(destinationPath, bytes) + Files.copy( + getResource("binaries/$name"), + destinationPath + ) destinationFile.setExecutable(true) } diff --git a/test_runner/src/test/kotlin/Debug.kt b/test_runner/src/test/kotlin/Debug.kt index f1c8c05f90..3b1e592633 100644 --- a/test_runner/src/test/kotlin/Debug.kt +++ b/test_runner/src/test/kotlin/Debug.kt @@ -10,8 +10,8 @@ fun main() { // run "gradle check" to generate required fixtures val projectId = System.getenv("GOOGLE_CLOUD_PROJECT") ?: "YOUR PROJECT ID" - val quantity = "single" - val type = "robo" + val quantity = "multiple" + val type = "error" // Bugsnag keeps the process alive so we must call exitProcess // https://github.com/bugsnag/bugsnag-java/issues/151 diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt index 9c4bac9192..d07981606d 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt @@ -26,6 +26,7 @@ import org.junit.Test import org.junit.rules.ExpectedException import org.junit.runner.RunWith import picocli.CommandLine +import java.io.StringReader @Suppress("TooManyFunctions") @RunWith(FlankTestRunner::class) @@ -1242,3 +1243,5 @@ AndroidArgs runBlocking { runAndroidTests(parsedYml) } } } + +private fun AndroidArgs.Companion.load(yamlData: String, cli: AndroidRunCommand? = null): AndroidArgs = load(StringReader(yamlData), cli) diff --git a/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt index bf647b605f..98bc47f3b0 100644 --- a/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt @@ -24,6 +24,7 @@ import org.junit.contrib.java.lang.system.SystemErrRule import org.junit.rules.ExpectedException import org.junit.runner.RunWith import picocli.CommandLine +import java.io.StringReader @Suppress("TooManyFunctions") @RunWith(FlankTestRunner::class) @@ -876,3 +877,5 @@ IosArgs assertFalse(iosArgs.keepFilePath) } } + +private fun IosArgs.Companion.load(yamlData: String, cli: IosRunCommand? = null): IosArgs = load(StringReader(yamlData), cli) diff --git a/test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt b/test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt index e805bdef13..c24000662d 100644 --- a/test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt @@ -6,6 +6,7 @@ import org.junit.Rule import org.junit.Test import org.junit.contrib.java.lang.system.SystemOutRule import org.junit.runner.RunWith +import java.io.StringReader @RunWith(FlankTestRunner::class) class YamlDeprecatedTest { @@ -134,3 +135,5 @@ class YamlDeprecatedTest { assertThat(output).isEqualTo(expected) } } + +private fun YamlDeprecated.modify(yamlData: String): Pair = modify(StringReader(yamlData)) diff --git a/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt b/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt index 92d6725919..6ff6ccd918 100644 --- a/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt +++ b/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt @@ -2,11 +2,13 @@ package ftl.doctor import com.google.common.truth.Truth.assertThat import ftl.args.AndroidArgs +import ftl.args.IArgsCompanion import ftl.args.IosArgs import ftl.test.util.FlankTestRunner import java.nio.file.Paths import org.junit.Test import org.junit.runner.RunWith +import java.io.StringReader @RunWith(FlankTestRunner::class) class DoctorTest { @@ -145,3 +147,5 @@ flank: assertThat(lint).isEqualTo("") } } + +private fun Doctor.validateYaml(args: IArgsCompanion, data: String): String = validateYaml(args, StringReader(data))