User memoized supplier pattern to reduce resource consumption
It's pointless to retrieve the whole data from the database and git backends when it's thrown away anyway. Use memoized supplier pattern to only retrieve the data if really needed. We need memoization in this context to ensure, that when the data is retrieved, it is done only once and not multiple times in different plugins and in hooks firing tool chain. This change breaks plugins that consume change events. However, given that only few plugins, e.g. reviewers, are affected by this change, this API change is justfied, as it's substantially decreasing the load on the gerrit site. One side effect of this change is stream event JSON serialization. Use custom JSON serializer to resolve supplier with their content. Test Plan: Non retrieval of data wth default gerrit install + reviewers plugin. 1. install gerrit with all core plugins 2. install reviewers plugin 3. create a group_100 that contains 100 users 4. add a reviewer to a change and select the group_100 Without this change hundreds of SQL select statements are executed in vain. That's because 100 ChangeEvents instances are created and multiple SQL select statements are executed per instance. With this change exact 0 select SQL statements are executed. Memoized suppliers are created but not used. Change-Id: I10e257be37ff60276e4187659926fe32e5be136b Reported-By: Luca Milanesio <luca.milanesio@gmail.com>
This commit is contained in:
committed by
Shawn Pearce
parent
12380e0a0d
commit
2605168713
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class ChangeAbandonedEvent extends PatchSetEvent {
|
||||
public AccountAttribute abandoner;
|
||||
public Supplier<AccountAttribute> abandoner;
|
||||
public String reason;
|
||||
|
||||
public ChangeAbandonedEvent() {
|
||||
|
||||
@@ -14,13 +14,14 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.server.data.ChangeAttribute;
|
||||
|
||||
public abstract class ChangeEvent extends RefEvent {
|
||||
public ChangeAttribute change;
|
||||
public Supplier<ChangeAttribute> change;
|
||||
|
||||
protected ChangeEvent(String type) {
|
||||
super(type);
|
||||
@@ -28,15 +29,15 @@ public abstract class ChangeEvent extends RefEvent {
|
||||
|
||||
@Override
|
||||
public Project.NameKey getProjectNameKey() {
|
||||
return new Project.NameKey(change.project);
|
||||
return new Project.NameKey(change.get().project);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getRefName() {
|
||||
return RefNames.fullName(change.branch);
|
||||
return RefNames.fullName(change.get().branch);
|
||||
}
|
||||
|
||||
public Change.Key getChangeKey() {
|
||||
return new Change.Key(change.id);
|
||||
return new Change.Key(change.get().id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class ChangeMergedEvent extends PatchSetEvent {
|
||||
public AccountAttribute submitter;
|
||||
public Supplier<AccountAttribute> submitter;
|
||||
public String newRev;
|
||||
|
||||
public ChangeMergedEvent() {
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class ChangeRestoredEvent extends PatchSetEvent {
|
||||
public AccountAttribute restorer;
|
||||
public Supplier<AccountAttribute> restorer;
|
||||
public String reason;
|
||||
|
||||
public ChangeRestoredEvent () {
|
||||
|
||||
@@ -14,12 +14,13 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
import com.google.gerrit.server.data.ApprovalAttribute;
|
||||
|
||||
public class CommentAddedEvent extends PatchSetEvent {
|
||||
public AccountAttribute author;
|
||||
public ApprovalAttribute[] approvals;
|
||||
public Supplier<AccountAttribute> author;
|
||||
public Supplier<ApprovalAttribute[]> approvals;
|
||||
public String comment;
|
||||
|
||||
public CommentAddedEvent() {
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class DraftPublishedEvent extends PatchSetEvent {
|
||||
public AccountAttribute uploader;
|
||||
public Supplier<AccountAttribute> uploader;
|
||||
|
||||
public DraftPublishedEvent() {
|
||||
super("draft-published");
|
||||
|
||||
@@ -61,6 +61,7 @@ import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
|
||||
import com.google.gerrit.server.query.change.ChangeData;
|
||||
import com.google.gerrit.server.query.change.InternalChangeQuery;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.gwtorm.server.SchemaFactory;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import com.google.inject.Singleton;
|
||||
@@ -92,6 +93,7 @@ public class EventFactory {
|
||||
private final ApprovalsUtil approvalsUtil;
|
||||
private final ChangeKindCache changeKindCache;
|
||||
private final Provider<InternalChangeQuery> queryProvider;
|
||||
private final SchemaFactory<ReviewDb> schema;
|
||||
|
||||
@Inject
|
||||
EventFactory(AccountCache accountCache,
|
||||
@@ -102,7 +104,8 @@ public class EventFactory {
|
||||
ChangeData.Factory changeDataFactory,
|
||||
ApprovalsUtil approvalsUtil,
|
||||
ChangeKindCache changeKindCache,
|
||||
Provider<InternalChangeQuery> queryProvider) {
|
||||
Provider<InternalChangeQuery> queryProvider,
|
||||
SchemaFactory<ReviewDb> schema) {
|
||||
this.accountCache = accountCache;
|
||||
this.urlProvider = urlProvider;
|
||||
this.patchListCache = patchListCache;
|
||||
@@ -112,6 +115,7 @@ public class EventFactory {
|
||||
this.approvalsUtil = approvalsUtil;
|
||||
this.changeKindCache = changeKindCache;
|
||||
this.queryProvider = queryProvider;
|
||||
this.schema = schema;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -121,6 +125,23 @@ public class EventFactory {
|
||||
* @param change
|
||||
* @return object suitable for serialization to JSON
|
||||
*/
|
||||
public ChangeAttribute asChangeAttribute(Change change) {
|
||||
try (ReviewDb db = schema.open()) {
|
||||
return asChangeAttribute(db, change);
|
||||
} catch (OrmException e) {
|
||||
log.error("Cannot open database connection", e);
|
||||
return new ChangeAttribute();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a ChangeAttribute for the given change suitable for serialization to
|
||||
* JSON.
|
||||
*
|
||||
* @param db Review database
|
||||
* @param change
|
||||
* @return object suitable for serialization to JSON
|
||||
*/
|
||||
public ChangeAttribute asChangeAttribute(ReviewDb db, Change change) {
|
||||
ChangeAttribute a = new ChangeAttribute();
|
||||
a.project = change.getProject().get();
|
||||
@@ -429,6 +450,19 @@ public class EventFactory {
|
||||
* @param patchSet
|
||||
* @return object suitable for serialization to JSON
|
||||
*/
|
||||
public PatchSetAttribute asPatchSetAttribute(RevWalk revWalk,
|
||||
PatchSet patchSet) {
|
||||
return asPatchSetAttribute(revWalk, patchSet);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a PatchSetAttribute for the given patchset suitable for
|
||||
* serialization to JSON.
|
||||
*
|
||||
* @param db Review database
|
||||
* @param patchSet
|
||||
* @return object suitable for serialization to JSON
|
||||
*/
|
||||
public PatchSetAttribute asPatchSetAttribute(ReviewDb db, RevWalk revWalk,
|
||||
PatchSet patchSet) {
|
||||
PatchSetAttribute p = new PatchSetAttribute();
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class HashtagsChangedEvent extends ChangeEvent {
|
||||
public AccountAttribute editor;
|
||||
public Supplier<AccountAttribute> editor;
|
||||
public String[] added;
|
||||
public String[] removed;
|
||||
public String[] hashtags;
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class MergeFailedEvent extends PatchSetEvent {
|
||||
public AccountAttribute submitter;
|
||||
public Supplier<AccountAttribute> submitter;
|
||||
public String reason;
|
||||
|
||||
public MergeFailedEvent() {
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class PatchSetCreatedEvent extends PatchSetEvent {
|
||||
public AccountAttribute uploader;
|
||||
public Supplier<AccountAttribute> uploader;
|
||||
|
||||
public PatchSetCreatedEvent() {
|
||||
super("patchset-created");
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.PatchSetAttribute;
|
||||
|
||||
public class PatchSetEvent extends ChangeEvent {
|
||||
public PatchSetAttribute patchSet;
|
||||
public Supplier<PatchSetAttribute> patchSet;
|
||||
|
||||
protected PatchSetEvent(String type) {
|
||||
super(type);
|
||||
|
||||
@@ -14,13 +14,14 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
import com.google.gerrit.server.data.RefUpdateAttribute;
|
||||
|
||||
public class RefUpdatedEvent extends RefEvent {
|
||||
public AccountAttribute submitter;
|
||||
public RefUpdateAttribute refUpdate;
|
||||
public Supplier<AccountAttribute> submitter;
|
||||
public Supplier<RefUpdateAttribute> refUpdate;
|
||||
|
||||
public RefUpdatedEvent() {
|
||||
super("ref-updated");
|
||||
@@ -28,11 +29,11 @@ public class RefUpdatedEvent extends RefEvent {
|
||||
|
||||
@Override
|
||||
public Project.NameKey getProjectNameKey() {
|
||||
return new Project.NameKey(refUpdate.project);
|
||||
return new Project.NameKey(refUpdate.get().project);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getRefName() {
|
||||
return refUpdate.refName;
|
||||
return refUpdate.get().refName;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class ReviewerAddedEvent extends PatchSetEvent {
|
||||
public AccountAttribute reviewer;
|
||||
public Supplier<AccountAttribute> reviewer;
|
||||
|
||||
public ReviewerAddedEvent() {
|
||||
super("reviewer-added");
|
||||
|
||||
@@ -14,10 +14,11 @@
|
||||
|
||||
package com.google.gerrit.server.events;
|
||||
|
||||
import com.google.common.base.Supplier;
|
||||
import com.google.gerrit.server.data.AccountAttribute;
|
||||
|
||||
public class TopicChangedEvent extends ChangeEvent {
|
||||
public AccountAttribute changer;
|
||||
public Supplier<AccountAttribute> changer;
|
||||
public String oldTopic;
|
||||
|
||||
public TopicChangedEvent() {
|
||||
|
||||
Reference in New Issue
Block a user