Skip to content

Commit a964b78

Browse files
vladakvladak
committed
use history cache when getting latest revision
1 parent 3ec7820 commit a964b78

File tree

7 files changed

+109
-59
lines changed

7 files changed

+109
-59
lines changed

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ public void accept(String sinceRevision, Consumer<String> visitor) throws Histor
469469
@Override
470470
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
471471
History hist = getHistory(file, null, null, 1);
472-
return getLastHistoryEntry(hist);
472+
return hist.getLastHistoryEntry();
473473
}
474474

475475
public History getHistory(File file, String sinceRevision, String tillRevision) throws HistoryException {

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
*/
2424
package org.opengrok.indexer.history;
2525

26+
import org.jetbrains.annotations.Nullable;
27+
2628
import java.io.Serializable;
2729
import java.util.ArrayList;
2830
import java.util.Collections;
@@ -179,6 +181,18 @@ public void strip() {
179181
tags.clear();
180182
}
181183

184+
/**
185+
* @return last (newest) history entry or null
186+
*/
187+
public @Nullable HistoryEntry getLastHistoryEntry() {
188+
List<HistoryEntry> historyEntries = getHistoryEntries();
189+
if (historyEntries == null || historyEntries.isEmpty()) {
190+
return null;
191+
}
192+
193+
return historyEntries.get(0);
194+
}
195+
182196
@Override
183197
public boolean equals(Object o) {
184198
if (this == o) {

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java

Lines changed: 74 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, 2020, Chris Fraire <[email protected]>.
2323
*/
2424
package org.opengrok.indexer.history;
@@ -46,6 +46,7 @@
4646
import java.util.logging.Logger;
4747
import java.util.stream.Collectors;
4848

49+
import org.jetbrains.annotations.Nullable;
4950
import org.opengrok.indexer.configuration.CommandTimeoutType;
5051
import org.opengrok.indexer.configuration.Configuration.RemoteSCM;
5152
import org.opengrok.indexer.configuration.PathAccepter;
@@ -228,50 +229,92 @@ public History getHistoryUI(File file) throws HistoryException {
228229
return getHistory(file, true, true);
229230
}
230231

231-
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
232-
final Repository repo = getRepository(file);
233-
if (repo != null) {
234-
return repo.getLastHistoryEntry(file, ui);
232+
private boolean isRepoHistoryEligible(Repository repo, File file, boolean ui) {
233+
RemoteSCM rscm = env.getRemoteScmSupported();
234+
boolean doRemote = (ui && (rscm == RemoteSCM.UIONLY))
235+
|| (rscm == RemoteSCM.ON)
236+
|| (ui || ((rscm == RemoteSCM.DIRBASED) && (repo != null) && repo.hasHistoryForDirectories()));
237+
238+
return (repo != null && repo.isHistoryEnabled() && repo.isWorking() && repo.fileHasHistory(file)
239+
&& (!repo.isRemote() || doRemote));
240+
}
241+
242+
@Nullable
243+
private History getHistoryFromCache(File file, Repository repository, boolean withFiles, boolean ui)
244+
throws HistoryException, ForbiddenSymlinkException {
245+
246+
if (useCache() && historyCache.supportsRepository(repository)) {
247+
return historyCache.get(file, repository, withFiles);
235248
}
249+
236250
return null;
237251
}
238252

239253
/**
240-
* Get the history for the specified file.
254+
* @param file file to get the history entry for
255+
* @param ui is the request coming from the UI
256+
* @return last (newest) history entry for given file or null
257+
* @throws HistoryException if history retrieval failed
258+
*/
259+
@Nullable
260+
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
261+
final File dir = file.isDirectory() ? file : file.getParentFile();
262+
final Repository repository = getRepository(dir);
263+
264+
if (!isRepoHistoryEligible(repository, file, ui)) {
265+
return null;
266+
}
267+
268+
History history;
269+
try {
270+
history = getHistoryFromCache(file, repository, false, ui);
271+
if (history != null) {
272+
HistoryEntry lastHistoryEntry = history.getLastHistoryEntry();
273+
if (lastHistoryEntry != null) {
274+
LOGGER.log(Level.FINEST, "got latest history entry {0} for ''{1}'' from history cache",
275+
new Object[]{lastHistoryEntry, file});
276+
return lastHistoryEntry;
277+
}
278+
}
279+
} catch (ForbiddenSymlinkException e) {
280+
LOGGER.log(Level.FINER, e.getMessage());
281+
return null;
282+
}
283+
284+
return repository.getLastHistoryEntry(file, ui);
285+
}
286+
287+
/**
288+
* Get the history for the specified file. The history cache is tried first, then the repository.
241289
*
242290
* @param file the file to get the history for
243-
* @param withFiles whether or not the returned history should contain a
244-
* list of files touched by each changeset (the file list may be skipped if
245-
* false, but it doesn't have to)
291+
* @param withFiles whether the returned history should contain a
292+
* list of files touched by each changeset (the file list may be skipped if false, but it doesn't have to)
246293
* @param ui called from the webapp
247294
* @return history for the file
248295
* @throws HistoryException on error when accessing the history
249296
*/
250-
public History getHistory(File file, boolean withFiles, boolean ui)
251-
throws HistoryException {
252-
final File dir = file.isDirectory() ? file : file.getParentFile();
253-
final Repository repo = getRepository(dir);
297+
public History getHistory(File file, boolean withFiles, boolean ui) throws HistoryException {
254298

255-
RemoteSCM rscm = env.getRemoteScmSupported();
256-
boolean doRemote = (ui && (rscm == RemoteSCM.UIONLY))
257-
|| (rscm == RemoteSCM.ON)
258-
|| (ui || ((rscm == RemoteSCM.DIRBASED) && (repo != null) && repo.hasHistoryForDirectories()));
299+
final File dir = file.isDirectory() ? file : file.getParentFile();
300+
final Repository repository = getRepository(dir);
259301

260-
if (repo != null && repo.isHistoryEnabled() && repo.isWorking() && repo.fileHasHistory(file)
261-
&& (!repo.isRemote() || doRemote)) {
302+
if (!isRepoHistoryEligible(repository, file, ui)) {
303+
return null;
304+
}
262305

263-
if (useCache() && historyCache.supportsRepository(repo)) {
264-
try {
265-
return historyCache.get(file, repo, withFiles);
266-
} catch (ForbiddenSymlinkException ex) {
267-
LOGGER.log(Level.FINER, ex.getMessage());
268-
return null;
269-
}
306+
History history;
307+
try {
308+
history = getHistoryFromCache(file, repository, withFiles, ui);
309+
if (history != null) {
310+
return history;
270311
}
271-
return repo.getHistory(file);
312+
} catch (ForbiddenSymlinkException e) {
313+
LOGGER.log(Level.FINER, e.getMessage());
314+
return null;
272315
}
273316

274-
return null;
317+
return repository.getHistory(file);
275318
}
276319

277320
/**
@@ -732,8 +775,9 @@ private List<Repository> getReposFromString(Collection<String> repositories) {
732775
return repos;
733776
}
734777

735-
protected Repository getRepository(File file) {
736-
return repositoryLookup.getRepository(file.toPath(), repositoryRoots.keySet(), repositories, PathUtils::getRelativeToCanonical);
778+
private Repository getRepository(File file) {
779+
return repositoryLookup.getRepository(file.toPath(), repositoryRoots.keySet(), repositories,
780+
PathUtils::getRelativeToCanonical);
737781
}
738782

739783
/**

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,9 +571,10 @@ public void accept(String sinceRevision, Consumer<String> visitor) throws Histor
571571
@Override
572572
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
573573
History hist = getHistory(file, null, null, 1);
574-
return getLastHistoryEntry(hist);
574+
return hist.getLastHistoryEntry();
575575
}
576576

577+
@Override
577578
History getHistory(File file, String sinceRevision) throws HistoryException {
578579
return getHistory(file, sinceRevision, null);
579580
}

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2008, 2021, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2008, 2022, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, 2020, Chris Fraire <[email protected]>.
2323
*/
2424
package org.opengrok.indexer.history;
@@ -50,8 +50,6 @@
5050
import org.opengrok.indexer.util.BufferSink;
5151
import org.opengrok.indexer.util.Executor;
5252

53-
import org.jetbrains.annotations.NotNull;
54-
5553
/**
5654
* An interface for an external repository.
5755
*
@@ -96,7 +94,6 @@ public abstract class Repository extends RepositoryInfo {
9694
*
9795
* @return {@code true} if the repository can get history for directories
9896
*/
99-
@NotNull
10097
abstract boolean hasHistoryForDirectories();
10198

10299
public String toString() {
@@ -132,19 +129,6 @@ public String toString() {
132129
*/
133130
abstract History getHistory(File file) throws HistoryException;
134131

135-
HistoryEntry getLastHistoryEntry(History hist) {
136-
if (hist == null) {
137-
return null;
138-
}
139-
140-
List<HistoryEntry> hlist = hist.getHistoryEntries();
141-
if (hlist == null || hlist.isEmpty()) {
142-
return null;
143-
}
144-
145-
return hlist.get(0);
146-
}
147-
148132
/**
149133
* This is generic implementation that retrieves the full history of given file
150134
* and returns the latest history entry. This is obviously very inefficient, both in terms of memory and I/O.
@@ -154,15 +138,19 @@ HistoryEntry getLastHistoryEntry(History hist) {
154138
* @throws HistoryException on error
155139
*/
156140
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
157-
History hist;
141+
History history;
158142
try {
159-
hist = HistoryGuru.getInstance().getHistory(file, false, ui);
143+
history = HistoryGuru.getInstance().getHistory(file, false, ui);
160144
} catch (HistoryException ex) {
161145
LOGGER.log(Level.WARNING, "failed to get history for {0}", file);
162146
return null;
163147
}
164148

165-
return getLastHistoryEntry(hist);
149+
if (history != null) {
150+
return history.getLastHistoryEntry();
151+
} else {
152+
return null;
153+
}
166154
}
167155

168156
public Repository() {
@@ -519,7 +507,7 @@ public List<String> getIgnoredDirs() {
519507
/**
520508
* Determine and return the current version of the repository.
521509
*
522-
* This operation is consider "heavy" so this function should not be
510+
* This operation is considered "heavy" so this function should not be
523511
* called on every web request.
524512
*
525513
* @param cmdType command timeout type

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryLookup.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ interface PathCanonicalizer {
6060
/**
6161
* Find enclosing repository for a given path.
6262
*
63-
* @param path path to find encolsing repository for
63+
* @param path path to find enclosing repository for
6464
* @param repoParentDirs Set of repository parent dirs (parents of repository roots)
6565
* @param repositories Map of repository root to Repository
6666
* @param canonicalizer PathCanonicalizer reference
@@ -77,7 +77,7 @@ Repository getRepository(Path path, Set<String> repoParentDirs, Map<String, Repo
7777
/**
7878
* Lifecycle method to invalidate any cache entries that point to given repositories that are being removed.
7979
*
80-
* @param removedRepos
80+
* @param removedRepos collection of repositories
8181
*/
8282
void repositoriesRemoved(Collection<Repository> removedRepos);
8383
}

‎opengrok-web/src/main/java/org/opengrok/web/PageConfig.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2011, Jens Elkner.
2323
* Portions Copyright (c) 2017, 2020, Chris Fraire <[email protected]>.
2424
*/
@@ -1288,17 +1288,20 @@ public File findDataFile() {
12881288
/**
12891289
* @return last revision string for {@code file} or null
12901290
*/
1291+
@Nullable
12911292
public String getLatestRevision() {
12921293
if (!getEnv().isHistoryEnabled()) {
12931294
return null;
12941295
}
12951296

1297+
// Try getting the history revision from the index first.
12961298
String lastRev = getLastRevFromIndex();
12971299
if (lastRev != null) {
12981300
return lastRev;
12991301
}
13001302

1301-
// fallback
1303+
// If this is older index, fallback to the history (either fetch from history cache or retrieve from
1304+
// the repository directly).
13021305
try {
13031306
return getLastRevFromHistory();
13041307
} catch (HistoryException e) {
@@ -1309,8 +1312,8 @@ public String getLatestRevision() {
13091312

13101313
@Nullable
13111314
private String getLastRevFromHistory() throws HistoryException {
1312-
HistoryEntry he = HistoryGuru.getInstance().
1313-
getLastHistoryEntry(new File(getEnv().getSourceRootFile(), getPath()), true);
1315+
File file = new File(getEnv().getSourceRootFile(), getPath());
1316+
HistoryEntry he = HistoryGuru.getInstance().getLastHistoryEntry(file, true);
13141317
if (he != null) {
13151318
return he.getRevision();
13161319
}

0 commit comments

Comments
 (0)