Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some minor peformance improvements. #1019

Merged
merged 1 commit into from
Jan 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions src/main/java/net/fabricmc/loom/task/RemapJarTask.java
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
});
}

Expand Down
21 changes: 11 additions & 10 deletions src/main/java/net/fabricmc/loom/util/download/Download.java
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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: [
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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") {
Expand Down