From 480dd5e393c29cad65229743098f3d79f56449ac Mon Sep 17 00:00:00 2001 From: modmuss Date: Sat, 6 Jan 2024 17:17:26 +0000 Subject: [PATCH] Some minor peformance improvements (#1019) --- .../net/fabricmc/loom/task/RemapJarTask.java | 11 ++--- .../fabricmc/loom/util/download/Download.java | 21 ++++---- .../test/benchmark/FabricAPIBenchmark.groovy | 12 ++++- .../unit/download/DownloadFileTest.groovy | 48 ++++++++++++++++++- 4 files changed, 72 insertions(+), 20 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index cb302af09..0948b353e 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2021-2022 FabricMC + * Copyright (c) 2021-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -124,12 +124,9 @@ private void setupPreparationTask() { mustRunAfter(prepareJarTask); getProject().getGradle().allprojects(project -> { - project.getTasks().configureEach(task -> { - if (task instanceof PrepareJarRemapTask otherTask) { - // Ensure that all remap jars run after all prepare tasks - mustRunAfter(otherTask); - } - }); + project.getTasks() + .withType(PrepareJarRemapTask.class) + .configureEach(this::mustRunAfter); }); } diff --git a/src/main/java/net/fabricmc/loom/util/download/Download.java b/src/main/java/net/fabricmc/loom/util/download/Download.java index 79c200cc0..bdd9b04b6 100644 --- a/src/main/java/net/fabricmc/loom/util/download/Download.java +++ b/src/main/java/net/fabricmc/loom/util/download/Download.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -43,7 +43,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; -import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.FileTime; import java.time.Duration; import java.time.Instant; @@ -191,6 +190,13 @@ private void doDownload(Path output) throws DownloadException { boolean success = statusCode == HttpURLConnection.HTTP_NOT_MODIFIED || (statusCode >= 200 && statusCode < 300); if (statusCode == HttpURLConnection.HTTP_NOT_MODIFIED) { + try { + // Update the last modified time so we don't retry the request until the max age has passed again. + Files.setLastModifiedTime(output, FileTime.from(Instant.now())); + } catch (IOException e) { + throw error(e, "Failed to update last modified time"); + } + // Success, etag matched. return; } @@ -365,9 +371,9 @@ private boolean isHashValid(Path path) { private boolean isOutdated(Path path) throws DownloadException { try { - final FileTime lastModified = getLastModified(path); - return lastModified.toInstant().plus(maxAge) - .isBefore(Instant.now()); + final FileTime lastModified = Files.getLastModifiedTime(path); + return lastModified.toInstant() + .isBefore(Instant.now().minus(maxAge)); } catch (IOException e) { throw error(e, "Failed to check if (%s) is outdated", path); } @@ -430,11 +436,6 @@ private static boolean exists(Path path) { return path.getFileSystem() == FileSystems.getDefault() ? path.toFile().exists() : Files.exists(path); } - private FileTime getLastModified(Path path) throws IOException { - final BasicFileAttributeView basicView = Files.getFileAttributeView(path, BasicFileAttributeView.class); - return basicView.readAttributes().lastModifiedTime(); - } - private Path getLockFile(Path output) { return output.resolveSibling(output.getFileName() + ".lock"); } diff --git a/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy b/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy index cad874f39..6857224da 100644 --- a/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -44,12 +44,20 @@ class FabricAPIBenchmark implements GradleProjectTestTrait { allowExistingRepo: true, repo: "https://github.com/FabricMC/fabric.git", - commit: "2facd446984085376bd23245410ebf2dc0881b02", + commit: "efa5891941a32589207dc58c2e77183d599465b8", patch: "fabric_api" ) gradle.enableMultiProjectOptimisation() + if (!gradle.buildGradle.text.contains("loom.mixin.useLegacyMixinAp")) { + gradle.buildGradle << """ + allprojects { + loom.mixin.useLegacyMixinAp = false + } + """.stripIndent() + } + def timeStart = new Date() def result = gradle.run(tasks: [ diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy index 1d9772035..cb3edfc31 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -234,6 +234,52 @@ class DownloadFileTest extends DownloadTest { requestCount == 1 } + def "ETag with max age"() { + setup: + // The count of requests, can return early if the ETag matches. + int requestCount = 0 + // The count of full downloads + int downloadCount = 0 + + server.get("/etag") { + def clientEtag = it.req.getHeader("If-None-Match") + + def result = "Hello world" + def etag = result.hashCode().toString() + it.header("ETag", etag) + + requestCount ++ + + if (clientEtag == etag) { + // Etag matches, no need to send the data. + it.status(HttpStatus.NOT_MODIFIED) + return + } + + it.result(result) + downloadCount ++ + } + + def output = new File(File.createTempDir(), "etag.txt").toPath() + + when: + for (i in 0..<3) { + if (i == 2) { + // On the third request, set the file to be 2 days old, forcing the etag to be checked. + Files.setLastModifiedTime(output, FileTime.from(Instant.now() - Duration.ofDays(2))) + } + + Download.create("$PATH/etag") + .etag(true) + .maxAge(Duration.ofDays(1)) + .downloadPath(output) + } + + then: + requestCount == 2 + downloadCount == 1 + } + def "Progress: File"() { setup: server.get("/progressFile") {