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

Improve materialization speed #411

Closed
wants to merge 7 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -148,7 +148,11 @@ public void run(Path scratchPath) {
}

try {
var prInstance = new PullRequestInstance(scratchPath.resolve("pr"), pr, bot.ignoreStaleReviews());
var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
var prInstance = new PullRequestInstance(scratchPath.resolve("pr"),
new HostedRepositoryPool(seedPath),
pr,
bot.ignoreStaleReviews());
CheckRun.execute(this, pr, prInstance, comments, allReviews, activeReviews, labels, census);
} catch (IOException e) {
throw new UncheckedIOException(e);
@@ -78,7 +78,11 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
var sanitizedUrl = URLEncoder.encode(pr.repository().webUrl().toString(), StandardCharsets.UTF_8);
var path = scratchPath.resolve("pr.integrate").resolve(sanitizedUrl);

var prInstance = new PullRequestInstance(path, pr, bot.ignoreStaleReviews());
var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
var prInstance = new PullRequestInstance(path,
new HostedRepositoryPool(seedPath),
pr,
bot.ignoreStaleReviews());
var localHash = prInstance.commit(censusInstance.namespace(), censusInstance.configuration().census().domain(), null);

// Validate the target hash if requested
@@ -22,7 +22,7 @@
*/
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.forge.*;

import java.io.*;
import java.nio.file.Path;
@@ -63,7 +63,11 @@ public void run(Path scratchPath) {
return;
}
try {
var prInstance = new PullRequestInstance(scratchPath.resolve("labeler"), pr, bot.ignoreStaleReviews());
var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
var prInstance = new PullRequestInstance(scratchPath.resolve("labeler"),
new HostedRepositoryPool(seedPath),
pr,
bot.ignoreStaleReviews());
var newLabels = getLabels(prInstance);
var currentLabels = pr.labels().stream()
.filter(key -> bot.labelPatterns().containsKey(key))
@@ -28,6 +28,7 @@
import org.openjdk.skara.json.JSONValue;
import org.openjdk.skara.vcs.Hash;

import java.nio.file.Path;
import java.util.*;
import java.util.concurrent.*;
import java.util.logging.Logger;
@@ -45,6 +46,7 @@ class PullRequestBot implements Bot {
private final IssueProject issueProject;
private final boolean ignoreStaleReviews;
private final Pattern allowedTargetBranches;
private final Path seedStorage;
private final ConcurrentMap<Hash, Boolean> currentLabels;
private final PullRequestUpdateCache updateCache;
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr");
@@ -53,7 +55,7 @@ class PullRequestBot implements Bot {
Map<String, List<Pattern>> labelPatterns, Map<String, String> externalCommands,
Map<String, String> blockingLabels, Set<String> readyLabels,
Map<String, Pattern> readyComments, IssueProject issueProject, boolean ignoreStaleReviews,
Pattern allowedTargetBranches) {
Pattern allowedTargetBranches, Path seedStorage) {
remoteRepo = repo;
this.censusRepo = censusRepo;
this.censusRef = censusRef;
@@ -65,6 +67,7 @@ class PullRequestBot implements Bot {
this.readyComments = readyComments;
this.ignoreStaleReviews = ignoreStaleReviews;
this.allowedTargetBranches = allowedTargetBranches;
this.seedStorage = seedStorage;

this.currentLabels = new ConcurrentHashMap<>();
this.updateCache = new PullRequestUpdateCache();
@@ -180,4 +183,8 @@ boolean ignoreStaleReviews() {
Pattern allowedTargetBranches() {
return allowedTargetBranches;
}

Optional<Path> seedStorage() {
return Optional.ofNullable(seedStorage);
}
}
@@ -25,6 +25,7 @@
import org.openjdk.skara.forge.HostedRepository;
import org.openjdk.skara.issuetracker.IssueProject;

import java.nio.file.Path;
import java.util.*;
import java.util.regex.Pattern;

@@ -40,6 +41,7 @@ public class PullRequestBotBuilder {
private IssueProject issueProject = null;
private boolean ignoreStaleReviews = false;
private Pattern allowedTargetBranches = Pattern.compile(".*");
private Path seedStorage = null;

PullRequestBotBuilder() {
}
@@ -99,8 +101,14 @@ public PullRequestBotBuilder allowedTargetBranches(String allowedTargetBranches)
return this;
}

public PullRequestBotBuilder seedStorage(Path seedStorage) {
this.seedStorage = seedStorage;
return this;
}

public PullRequestBot build() {
return new PullRequestBot(repo, censusRepo, censusRef, labelPatterns, externalCommands, blockingLabels,
readyLabels, readyComments, issueProject, ignoreStaleReviews, allowedTargetBranches);
readyLabels, readyComments, issueProject, ignoreStaleReviews, allowedTargetBranches,
seedStorage);
}
}
@@ -73,7 +73,8 @@ public List<Bot> create(BotConfiguration configuration) {
.blockingLabels(blockers)
.readyLabels(readyLabels)
.readyComments(readyComments)
.externalCommands(external);
.externalCommands(external)
.seedStorage(configuration.storageFolder().resolve("seeds"));

if (repo.value().contains("labels")) {
var labelPatterns = new HashMap<String, List<Pattern>>();
@@ -43,16 +43,17 @@ class PullRequestInstance {
private final Hash baseHash;
private final boolean ignoreStaleReviews;

PullRequestInstance(Path localRepoPath, PullRequest pr, boolean ignoreStaleReviews) throws IOException {
PullRequestInstance(Path localRepoPath, HostedRepositoryPool hostedRepositoryPool, PullRequest pr, boolean ignoreStaleReviews) throws IOException {
this.pr = pr;
this.ignoreStaleReviews = ignoreStaleReviews;

// Materialize the PR's source and target ref
var repository = pr.repository();
localRepo = hostedRepositoryPool.checkout(pr, localRepoPath);
localRepo.fetch(repository.url(), "+" + pr.targetRef() + ":pr_prinstance");

// Materialize the PR's target ref
localRepo = Repository.materialize(localRepoPath, repository.url(),
"+" + pr.targetRef() + ":pr_prinstance_" + repository.name());
targetHash = localRepo.fetch(repository.url(), pr.targetRef());
headHash = localRepo.fetch(repository.url(), pr.headHash().hex());
targetHash = pr.targetHash();
headHash = pr.headHash();
baseHash = localRepo.mergeBase(targetHash, headHash);
}

@@ -22,7 +22,7 @@
*/
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.issuetracker.Comment;
import org.openjdk.skara.vcs.Hash;

@@ -73,7 +73,11 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
var sanitizedUrl = URLEncoder.encode(pr.repository().webUrl().toString(), StandardCharsets.UTF_8);
var path = scratchPath.resolve("pr.sponsor").resolve(sanitizedUrl);

var prInstance = new PullRequestInstance(path, pr, bot.ignoreStaleReviews());
var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
var prInstance = new PullRequestInstance(path,
new HostedRepositoryPool(seedPath),
pr,
bot.ignoreStaleReviews());
var localHash = prInstance.commit(censusInstance.namespace(), censusInstance.configuration().census().domain(),
comment.author().id());

@@ -0,0 +1,163 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.openjdk.skara.forge;

import org.openjdk.skara.vcs.*;

import java.io.*;
import java.nio.file.*;
import java.util.*;
import java.util.logging.Logger;

public class HostedRepositoryPool {
private final Path seedStorage;
private final Logger log = Logger.getLogger("org.openjdk.skara.forge");

public HostedRepositoryPool(Path seedStorage) {
this.seedStorage = seedStorage;
}

private class HostedRepositoryInstance {
private final HostedRepository hostedRepository;
private final Path seed;
private final String ref;

private HostedRepositoryInstance(HostedRepository hostedRepository, String ref) {
this.hostedRepository = hostedRepository;
this.seed = seedStorage.resolve(hostedRepository.name());
this.ref = ref;
}

private class NewClone {
private final Repository repository;
private final Hash fetchHead;

NewClone(Repository repository, Hash fetchHead) {
this.repository = repository;
this.fetchHead = fetchHead;
}

Repository repository() {
return repository;
}

Hash fetchHead() {
return fetchHead;
}
}

private void clearDirectory(Path directory) {
try {
Files.walk(directory)
.map(Path::toFile)
.sorted(Comparator.reverseOrder())
.forEach(File::delete);
} catch (IOException io) {
throw new RuntimeException(io);
}
}

private void initializeSeed() throws IOException {
if (!Files.exists(seed)) {
Files.createDirectories(seed.getParent());
var tmpSeedFolder = seed.resolveSibling(seed.getFileName().toString() + "-" + UUID.randomUUID());
Repository.clone(hostedRepository.url(), tmpSeedFolder, true);
try {
Files.move(tmpSeedFolder, seed);
log.info("Seeded repository " + hostedRepository.name() + " into " + seed);
} catch (IOException e) {
log.info("Failed to populate seed folder " + seed + " - perhaps due to a benign race. Ignoring..");
clearDirectory(tmpSeedFolder);
}
}
}

private Repository cloneSeeded(Path path) throws IOException {
initializeSeed();
log.info("Using seed folder " + seed + " when cloning into " + path);
return Repository.clone(hostedRepository.url(), path, false, seed);
}

private NewClone fetchRef(Repository repository) throws IOException {
var fetchHead = repository.fetch(hostedRepository.url(), "+" + ref + ":" + ref);
return new NewClone(repository, fetchHead);
}

private NewClone materializeClone(Path path) throws IOException {
var localRepo = Repository.get(path);
if (localRepo.isEmpty()) {
return fetchRef(cloneSeeded(path));
} else {
var localRepoInstance = localRepo.get();
if (!localRepoInstance.isHealthy()) {
var preserveUnhealthy = seed.resolveSibling(seed.getFileName().toString() + "-unhealthy-" + UUID.randomUUID());
log.severe("Unhealthy local repository detected - preserved in: " + preserveUnhealthy);
Files.move(localRepoInstance.root(), preserveUnhealthy);
return fetchRef(cloneSeeded(path));
} else {
try {
localRepoInstance.clean();
return fetchRef(localRepoInstance);
} catch (IOException e) {
var preserveUnclean = seed.resolveSibling(seed.getFileName().toString() + "-unclean-" + UUID.randomUUID());
log.severe("Uncleanable local repository detected - preserved in: " + preserveUnclean);
Files.move(localRepoInstance.root(), preserveUnclean);
return fetchRef(cloneSeeded(path));
}
}
}
}
}

public Repository materialize(HostedRepository hostedRepository, String ref, Path path) throws IOException {
var hostedRepositoryInstance = new HostedRepositoryInstance(hostedRepository, ref);
var clone = hostedRepositoryInstance.materializeClone(path);
return clone.repository();
}

public Repository materialize(PullRequest pr, Path path) throws IOException {
return materialize(pr.repository(), pr.sourceRef(), path);
}

public Repository checkout(HostedRepository hostedRepository, String ref, Path path) throws IOException {
var hostedRepositoryInstance = new HostedRepositoryInstance(hostedRepository, ref);
var clone = hostedRepositoryInstance.materializeClone(path);
var localRepo = clone.repository();
try {
localRepo.checkout(clone.fetchHead(), true);
} catch (IOException e) {
var preserveUnchecked = hostedRepositoryInstance.seed.resolveSibling(
hostedRepositoryInstance.seed.getFileName().toString() + "-unchecked-" + UUID.randomUUID());
log.severe("Uncheckoutable local repository detected - preserved in: " + preserveUnchecked);
Files.move(localRepo.root(), preserveUnchecked);
clone = hostedRepositoryInstance.fetchRef(hostedRepositoryInstance.cloneSeeded(path));
localRepo = clone.repository();
localRepo.checkout(clone.fetchHead(), true);
}
return localRepo;
}

public Repository checkout(PullRequest pr, Path path) throws IOException {
return checkout(pr.repository(), pr.sourceRef(), path);
}
}
@@ -60,6 +60,11 @@ public List<Branch> branches() throws IOException {
return branches;
}

@Override
public List<Branch> branches(String remote) throws IOException {
return branches;
}

void setBranches(List<Branch> branches) {
this.branches = branches;
}
@@ -28,7 +28,6 @@
import java.io.IOException;
import java.net.URI;
import java.nio.file.Path;
import java.nio.file.Files;
import java.time.ZonedDateTime;
import java.util.*;

@@ -194,13 +193,17 @@ static Repository clone(URI from, Path to) throws IOException {
}

static Repository clone(URI from, Path to, boolean isBare) throws IOException {
return from.getPath().toString().endsWith(".git") ?
GitRepository.clone(from, to, isBare) : HgRepository.clone(from, to, isBare);
return clone(from, to, isBare, null);
}

static Repository clone(URI from, Path to, boolean isBare, Path seed) throws IOException {
return from.getPath().endsWith(".git") ?
GitRepository.clone(from, to, isBare, seed) : HgRepository.clone(from, to, isBare, seed);
}

static Repository mirror(URI from, Path to) throws IOException {
return from.getPath().toString().endsWith(".git") ?
GitRepository.mirror(from, to) :
HgRepository.clone(from, to, true); // hg does not have concept of "mirror"
HgRepository.clone(from, to, true, null); // hg does not have concept of "mirror"
}
}