From 2edc2b3d06f5816ea25000f90a8eea5a27412303 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 10 Dec 2010 10:00:45 -0800 Subject: [PATCH] Speed up push by hiding refs/changes/ No user (even site administrators) is permitted to make modifications to the refs/changes/ namespace during push. This restriction prevents commits needed to remember patch sets of a change from being deleted or replaced after the review has already started. The primary reason the git receive-pack server advertises references to the send-pack client is to let it know what references the client can change, and allow the client to detect if the change should require the --force flag. The secondary reason is to help the client compute a common ancestor with the server, and reduce the amount of objects it must upload. Either way the client doesn't need the list of all commits submitted for review, it can function perfectly fine without the refs/changes/ names. For some really busy repositories, this can save megabytes worth of text that needs to be sent from server to client each time the client tries to push a change for review. Change-Id: I4815adcdb58b8fea24b77b6b5e92a0d073dddb0f Signed-off-by: Shawn O. Pearce --- .../google/gerrit/httpd/ProjectServlet.java | 2 +- .../com/google/gerrit/server/git/PushOp.java | 2 +- .../gerrit/server/git/ReceiveCommits.java | 3 +- .../server/git/ReceiveCommitsRefFilter.java | 41 +++++++++++++++++++ .../gerrit/server/git/VisibleRefFilter.java | 9 +++- .../google/gerrit/sshd/commands/Upload.java | 2 +- 6 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsRefFilter.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectServlet.java index 09e2cce8ef..8c1e5dfd30 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectServlet.java @@ -198,7 +198,7 @@ public class ProjectServlet extends GitServlet { UploadPack up = new UploadPack(repo); up.setPackConfig(packConfig); if (!pc.allRefsAreVisible()) { - up.setRefFilter(new VisibleRefFilter(repo, pc, db.get())); + up.setRefFilter(new VisibleRefFilter(repo, pc, db.get(), true)); } return up; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/PushOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/PushOp.java index 1c100dfc9a..043e7c4a17 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/PushOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/PushOp.java @@ -296,7 +296,7 @@ class PushOp implements ProjectRunnable { return Collections.emptyList(); } try { - local = new VisibleRefFilter(db, pc, meta).filter(local); + local = new VisibleRefFilter(db, pc, meta, true).filter(local); } finally { meta.close(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 710e97744d..a9979965fe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -208,8 +208,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { if (!projectControl.allRefsAreVisible()) { rp.setCheckReferencedObjectsAreReachable(true); - rp.setRefFilter(new VisibleRefFilter(repo, projectControl, db)); + rp.setRefFilter(new VisibleRefFilter(repo, projectControl, db, false)); } + rp.setRefFilter(new ReceiveCommitsRefFilter(rp.getRefFilter())); rp.setPreReceiveHook(this); rp.setPostReceiveHook(this); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsRefFilter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsRefFilter.java new file mode 100644 index 0000000000..730305ccee --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsRefFilter.java @@ -0,0 +1,41 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.git; + +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.transport.RefFilter; + +import java.util.HashMap; +import java.util.Map; + +/** Exposes only the non refs/changes/ reference names. */ +class ReceiveCommitsRefFilter implements RefFilter { + private final RefFilter base; + + public ReceiveCommitsRefFilter(RefFilter base) { + this.base = base != null ? base : RefFilter.DEFAULT; + } + + @Override + public Map filter(Map refs) { + HashMap r = new HashMap(); + for (Map.Entry e : refs.entrySet()) { + if (!e.getKey().startsWith("refs/changes/")) { + r.put(e.getKey(), e.getValue()); + } + } + return base.filter(r); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java index ffc3fd8e19..8e4d8eaa38 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java @@ -52,12 +52,15 @@ public class VisibleRefFilter implements RefFilter { private final Repository db; private final ProjectControl projectCtl; private final ReviewDb reviewDb; + private final boolean showChanges; public VisibleRefFilter(final Repository db, - final ProjectControl projectControl, final ReviewDb reviewDb) { + final ProjectControl projectControl, final ReviewDb reviewDb, + final boolean showChanges) { this.db = db; this.projectCtl = projectControl; this.reviewDb = reviewDb; + this.showChanges = showChanges; } @Override @@ -99,6 +102,10 @@ public class VisibleRefFilter implements RefFilter { } private Set visibleChanges() { + if (!showChanges) { + return Collections.emptySet(); + } + final Project project = projectCtl.getProject(); try { final Set visibleChanges = new HashSet(); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java index bcc9d19d32..95b33f74c1 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java @@ -42,7 +42,7 @@ final class Upload extends AbstractGitCommand { final UploadPack up = new UploadPack(repo); if (!projectControl.allRefsAreVisible()) { - up.setRefFilter(new VisibleRefFilter(repo, projectControl, db.get())); + up.setRefFilter(new VisibleRefFilter(repo, projectControl, db.get(), true)); } up.setPackConfig(config.getPackConfig()); up.setTimeout(config.getTimeout());