opendev/gerrit
Code Issues Proposed changes

Compute some logging args lazily

Browse Source 
This way no work is done at the log site to compute the logging args (it
only creates the instances for the lambda expressions). Flogger will
only evaluate the lambdas if it intends to actually log the message
which makes the logging statement virtually free if it is disabled.

There are likely more places where we can benefit from lazy logging
arguments. We should convert them whenever we find them.

Change-Id: Ic422a413fd76fcceb8102a7d1959e6c116e1f05f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-05-16 14:19:49 +02:00
parent 8860e8e0ad
commit 138573a899
3 changed files with 61 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
java/com/google/gerrit/common/SiteLibraryLoaderUtil.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.common;
import static com.google.common.flogger.LazyArgs.lazy;
import static com.google.gerrit.common.FileUtil.lastModified;
import static java.util.stream.Collectors.joining;
@@ -37,7 +38,7 @@ public final class SiteLibraryLoaderUtil {
try {
List<Path> jars = listJars(libdir);
IoUtil.loadJARs(jars);
logger.atFine().log("Loaded site libraries: %s", jarList(jars));
logger.atFine().log("Loaded site libraries: %s", lazy(() -> jarList(jars)));
} catch (IOException e) {
logger.atSevere().withCause(e).log("Error scanning lib directory %s", libdir);
}

4
java/com/google/gerrit/gpg/PublicKeyChecker.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.gpg;
import static com.google.common.flogger.LazyArgs.lazy;
import static com.google.gerrit.extensions.common.GpgKeyInfo.Status.BAD;
import static com.google.gerrit.extensions.common.GpgKeyInfo.Status.OK;
import static com.google.gerrit.extensions.common.GpgKeyInfo.Status.TRUSTED;
@@ -297,7 +298,8 @@ public class PublicKeyChecker {
.atInfo()
.log(
"Key %s is revoked by %s, which is not in the store. Assuming revocation is valid.",
Fingerprint.toString(key.getFingerprint()), Fingerprint.toString(rfp));
lazy(() -> Fingerprint.toString(key.getFingerprint())),
lazy(() -> Fingerprint.toString(rfp)));
problems.add(reasonToString(getRevocationReason(revocation)));
continue;
}

87
java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.change;
import static com.google.common.flogger.LazyArgs.lazy;
import static java.util.stream.Collectors.toList;
import com.google.common.base.Strings;
@@ -217,39 +218,16 @@ public class ReviewersUtil {
}
logger.atFine().log("Filtered recommendations: %s", filteredRecommendations);
List<SuggestedReviewerInfo> suggestedReviewers = loadAccounts(filteredRecommendations);
if (!excludeGroups && suggestedReviewers.size() < limit && !Strings.isNullOrEmpty(query)) {
// Add groups at the end as individual accounts are usually more
// important.
suggestedReviewers.addAll(
suggestAccountGroups(
suggestReviewers,
projectState,
visibilityControl,
limit - suggestedReviewers.size()));
}
if (suggestedReviewers.size() > limit) {
suggestedReviewers = suggestedReviewers.subList(0, limit);
logger.atFine().log("Limited suggested reviewers to %d accounts.", limit);
}
List<SuggestedReviewerInfo> suggestedReviewers =
suggestReviewers(
suggestReviewers,
projectState,
visibilityControl,
excludeGroups,
filteredRecommendations);
logger
.atFine()
.log(
"Suggested reviewers: %s",
suggestedReviewers
.stream()
.map(
r -> {
if (r.account != null) {
return "a/" + r.account._accountId;
} else if (r.group != null) {
return "g/" + r.group.id;
} else {
return "";
}
})
.collect(toList()));
.log("Suggested reviewers: %s", lazy(() -> formatSuggestedReviewers(suggestedReviewers)));
return suggestedReviewers;
}
@@ -283,6 +261,36 @@ public class ReviewersUtil {
}
}
private List<SuggestedReviewerInfo> suggestReviewers(
SuggestReviewers suggestReviewers,
ProjectState projectState,
VisibilityControl visibilityControl,
boolean excludeGroups,
List<Account.Id> filteredRecommendations)
throws OrmException, PermissionBackendException, IOException {
List<SuggestedReviewerInfo> suggestedReviewers = loadAccounts(filteredRecommendations);
int limit = suggestReviewers.getLimit();
if (!excludeGroups
&& suggestedReviewers.size() < limit
&& !Strings.isNullOrEmpty(suggestReviewers.getQuery())) {
// Add groups at the end as individual accounts are usually more
// important.
suggestedReviewers.addAll(
suggestAccountGroups(
suggestReviewers,
projectState,
visibilityControl,
limit - suggestedReviewers.size()));
}
if (suggestedReviewers.size() > limit) {
suggestedReviewers = suggestedReviewers.subList(0, limit);
logger.atFine().log("Limited suggested reviewers to %d accounts.", limit);
}
return suggestedReviewers;
}
private List<Account.Id> recommendAccounts(
@Nullable ChangeNotes changeNotes,
SuggestReviewers suggestReviewers,
@@ -414,4 +422,21 @@ public class ReviewersUtil {
return result;
}
private static String formatSuggestedReviewers(List<SuggestedReviewerInfo> suggestedReviewers) {
return suggestedReviewers
.stream()
.map(
r -> {
if (r.account != null) {
return "a/" + r.account._accountId;
} else if (r.group != null) {
return "g/" + r.group.id;
} else {
return "";
}
})
.collect(toList())
.toString();
}
}