From e829566ca86d7883d320776c0db73c8e225bceb5 Mon Sep 17 00:00:00 2001 From: Chris Fraire Date: Fri, 30 Nov 2018 21:56:57 -0600 Subject: [PATCH 1/4] Support ctags for historical revisions - Add webappCtags configuration flag, with --webappCtags switch, to indicate if the webapp is eligible to run ctags. - Add Repository.getHistoryGet() override to allow sub-classes to override to avoid a full in-memory version; and override for Git and Mercurial. - Revise BoundedBlockingObjectPool as LIFO-to-FIFO so the webapp does not start extraneous instances; Indexer switches to FIFO performance when the queue pool is emptied. - make IndexerParallelizer a lazy property of RuntimeEnvironment, and make the executors of IndexerParallelizer also lazy properties. Move the history-related executors into IndexerParallelizer so the lifecycle of all indexing/history executors are controlled in the same class. --- dev/checkstyle/suppressions.xml | 25 ++- .../indexer/configuration/Configuration.java | 16 ++ .../configuration/RuntimeEnvironment.java | 77 +++------ .../indexer/history/FileHistoryCache.java | 7 +- .../indexer/history/GitRepository.java | 74 +++++--- .../opengrok/indexer/history/HistoryGuru.java | 42 ++--- .../indexer/history/MercurialRepository.java | 72 ++++++-- .../opengrok/indexer/history/Repository.java | 34 +++- .../opengrok/indexer/index/IndexDatabase.java | 47 ++--- .../org/opengrok/indexer/index/Indexer.java | 48 +++--- .../indexer/index/IndexerParallelizer.java | 161 +++++++++++++++--- .../util/BoundedBlockingObjectPool.java | 53 ++++-- .../org/opengrok/indexer/util/BufferSink.java | 34 ++++ .../indexer/util/LazilyInstantiate.java | 111 ++++++++++++ .../indexer/index/IndexDatabaseTest.java | 10 +- .../opengrok/indexer/index/IndexerTest.java | 29 +--- .../java/org/opengrok/web/WebappListener.java | 7 +- opengrok-web/src/main/webapp/list.jsp | 66 ++++++- 18 files changed, 669 insertions(+), 244 deletions(-) create mode 100644 opengrok-indexer/src/main/java/org/opengrok/indexer/util/BufferSink.java create mode 100644 opengrok-indexer/src/main/java/org/opengrok/indexer/util/LazilyInstantiate.java diff --git a/dev/checkstyle/suppressions.xml b/dev/checkstyle/suppressions.xml index ca7544ab54b..e0833eeb48c 100644 --- a/dev/checkstyle/suppressions.xml +++ b/dev/checkstyle/suppressions.xml @@ -1,5 +1,27 @@ + @@ -9,7 +31,8 @@ |CustomExactPhraseScorer\.java|PhrasePositions\.java|PhraseQueue\.java|Summarizer\.java| |Summary\.java|OGKUnifiedHighlighter\.java|ResponseHeaderFilter\.java|BoundedBlockingObjectPool\.java| |ObjectPool\.java|ObjectValidator\.java|ObjectFactory\.java|AbstractObjectPool\.java| - |BlockingObjectPool\.java|OGKTextVecField\.java|OGKTextField\.java" /> + |BlockingObjectPool\.java|OGKTextVecField\.java|OGKTextField\.java| + |LazilyInstantiate\.java" /> lzIndexerParallelizer; + private final LazilyInstantiate lzSearchExecutor; private static final RuntimeEnvironment instance = new RuntimeEnvironment(); - private static ExecutorService historyExecutor = null; - private static ExecutorService historyRenamedExecutor = null; - private static ExecutorService searchExecutor = null; private final Map> repository_map = new ConcurrentHashMap<>(); private final Map searcherManagerMap = new ConcurrentHashMap<>(); @@ -127,43 +127,21 @@ private RuntimeEnvironment() { configuration = new Configuration(); configLock = new ReentrantReadWriteLock(); watchDog = new WatchDogService(); + lzIndexerParallelizer = LazilyInstantiate.using(() -> + new IndexerParallelizer(this)); + lzSearchExecutor = LazilyInstantiate.using(() -> newSearchExecutor()); } /** Instance of authorization framework.*/ private AuthorizationFramework authFramework; - /* Get thread pool used for top-level repository history generation. */ - public static synchronized ExecutorService getHistoryExecutor() { - if (historyExecutor == null) { - historyExecutor = Executors.newFixedThreadPool(getInstance().getHistoryParallelism(), - runnable -> { - Thread thread = Executors.defaultThreadFactory().newThread(runnable); - thread.setName("history-handling-" + thread.getId()); - return thread; - }); - } - - return historyExecutor; - } - - /* Get thread pool used for history generation of renamed files. */ - public static synchronized ExecutorService getHistoryRenamedExecutor() { - if (historyRenamedExecutor == null) { - historyRenamedExecutor = Executors.newFixedThreadPool(getInstance().getHistoryRenamedParallelism(), - runnable -> { - Thread thread = Executors.defaultThreadFactory().newThread(runnable); - thread.setName("renamed-handling-" + thread.getId()); - return thread; - }); - } - - return historyRenamedExecutor; + /** Gets the thread pool used for multi-project searches. */ + public ExecutorService getSearchExecutor() { + return lzSearchExecutor.get(); } - /* Get thread pool used for multi-project searches. */ - public synchronized ExecutorService getSearchExecutor() { - if (searchExecutor == null) { - searchExecutor = Executors.newFixedThreadPool( + private ExecutorService newSearchExecutor() { + return Executors.newFixedThreadPool( this.getMaxSearchThreadCount(), new ThreadFactory() { @Override @@ -173,23 +151,6 @@ public Thread newThread(Runnable runnable) { return thread; } }); - } - - return searchExecutor; - } - - public static synchronized void freeHistoryExecutor() { - historyExecutor = null; - } - - public static synchronized void destroyRenamedHistoryExecutor() throws InterruptedException { - if (historyRenamedExecutor != null) { - historyRenamedExecutor.shutdown(); - // All the jobs should be completed by now however for testing - // we would like to make sure the threads are gone. - historyRenamedExecutor.awaitTermination(1, TimeUnit.MINUTES); - historyRenamedExecutor = null; - } } /** @@ -203,6 +164,10 @@ public static RuntimeEnvironment getInstance() { public WatchDogService watchDog; + public IndexerParallelizer getIndexerParallelizer() { + return lzIndexerParallelizer.get(); + } + private String getCanonicalPath(String s) { if (s == null) { return null; } try { @@ -1092,6 +1057,14 @@ public void setWebappLAF(String laf) { setConfigurationValue("webappLAF", laf); } + /** + * Gets a value indicating if the web app should run ctags as necessary. + * @return the value of {@link Configuration#isWebappCtags()} + */ + public boolean isWebappCtags() { + return (boolean)getConfigurationValue("webappCtags"); + } + public Configuration.RemoteSCM getRemoteScmSupported() { return (Configuration.RemoteSCM)getConfigurationValue("remoteScmSupported"); } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java index 9bc87a35fae..4582f1139aa 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java @@ -333,7 +333,7 @@ private History mergeOldAndNewHistory(File cacheFile, History histNew, Repositor /** * Store history object (encoded as XML and compressed with gzip) in a file. * - * @param history history object to store + * @param histNew history object to store * @param file file to store the history object into * @param repo repository for the file * @param mergeHistory whether to merge the history with existing or @@ -536,9 +536,7 @@ public void store(History history, Repository repository) final CountDownLatch latch = new CountDownLatch(renamed_map.size()); AtomicInteger renamedFileHistoryCount = new AtomicInteger(); for (final Map.Entry> map_entry : renamed_map.entrySet()) { - RuntimeEnvironment.getHistoryRenamedExecutor().submit(new Runnable() { - @Override - public void run() { + env.getIndexerParallelizer().getHistoryRenamedExecutor().submit(() -> { try { doFileHistory(map_entry.getKey(), map_entry.getValue(), env, repositoryF, @@ -552,7 +550,6 @@ public void run() { } finally { latch.countDown(); } - } }); } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java index 5238773c21a..bd75f752c3f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.history; @@ -27,9 +27,11 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.OutputStream; import java.io.Reader; import java.nio.file.Paths; import java.text.ParseException; @@ -46,6 +48,7 @@ import java.util.regex.Pattern; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; import org.opengrok.indexer.util.StringUtils; @@ -171,14 +174,18 @@ Executor getRenamedFilesExecutor(final File file, String sinceRevision) throws I /** * Try to get file contents for given revision. * + * @param sink a required target sink + * @param outIterations a required out array for storing the number of calls + * made to {@code sink} * @param fullpath full pathname of the file * @param rev revision - * @return contents of the file in revision rev + * @return {@code true} if any contents were found */ - private InputStream getHistoryRev(String fullpath, String rev) { - InputStream ret = null; - File directory = new File(getDirectoryName()); + private boolean getHistoryRev(BufferSink sink, int[] outIterations, + String fullpath, String rev) { + outIterations[0] = 0; + File directory = new File(getDirectoryName()); try { /* * Be careful, git uses only forward slashes in its command and output (not in file path). @@ -197,13 +204,13 @@ private InputStream getHistoryRev(String fullpath, String rev) { RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); int status = executor.exec(); - ByteArrayOutputStream out = new ByteArrayOutputStream(); byte[] buffer = new byte[32 * 1024]; try (InputStream in = executor.getOutputStream()) { int len; while ((len = in.read(buffer)) != -1) { if (len > 0) { - out.write(buffer, 0, len); + outIterations[0]++; + sink.write(buffer, 0, len); } } } @@ -212,22 +219,48 @@ private InputStream getHistoryRev(String fullpath, String rev) { * If exit value of the process was not 0 then the file did * not exist or internal git error occured. */ - if (status == 0) { - ret = new ByteArrayInputStream(out.toByteArray()); - } else { - ret = null; - } + return status == 0; } catch (Exception exp) { LOGGER.log(Level.SEVERE, "Failed to get history for file {0} in revision {1}: ", new Object[]{fullpath, rev, exp.getClass().toString(), exp}); + return false; } + } - return ret; + /** + * Gets the contents of a specific version of a named file into the + * specified target without a full, in-memory buffer. + * + * @param target a required target file which will be overwritten + * @param parent the name of the directory containing the file + * @param basename the name of the file to get + * @param rev the revision to get + * @return {@code true} if contents were found + * @throws java.io.IOException if an I/O error occurs + */ + @Override + public boolean getHistoryGet(File target, String parent, String basename, + String rev) throws IOException { + try (OutputStream out = new FileOutputStream(target)) { + return getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), + parent, basename, rev); + } } @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { + public InputStream getHistoryGet(String parent, String basename, + String rev) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + if (getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), + parent, basename, rev)) { + return new ByteArrayInputStream(out.toByteArray()); + } + return null; + } + + protected boolean getHistoryGet(BufferSink sink, String parent, + String basename, String rev) { String fullpath; try { fullpath = new File(parent, basename).getCanonicalPath(); @@ -238,12 +271,13 @@ public String get() { return String.format("Failed to get canonical path: %s/%s", parent, basename); } }); - return null; + return false; } - InputStream ret = getHistoryRev(fullpath, rev); - - if (ret == null) { + int[] iterations = new int[1]; + boolean ret = getHistoryRev((buf, offset, n) -> sink.write(buf, offset, + n), iterations, fullpath, rev); + if (!ret && iterations[0] < 1) { /* * If we failed to get the contents it might be that the file was * renamed so we need to find its original name in that revision @@ -259,10 +293,10 @@ public String get() { return String.format("Failed to get original revision: %s/%s (revision %s)", parent, basename, rev); } }); - return null; + return false; } if (origpath != null) { - ret = getHistoryRev(origpath, rev); + ret = getHistoryRev(sink, iterations, origpath, rev); } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java index 5972a27aa99..9d72f94375c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java @@ -42,7 +42,6 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -267,6 +266,24 @@ public History getHistory(File file, boolean withFiles, boolean ui) return null; } + /** + * Gets a named revision of the specified file into the specified target. + * + * @param target a require target file + * @param parent The directory containing the file + * @param basename The name of the file + * @param rev The revision to get + * @return {@code true} if content was found + * @throws java.io.IOException if an I/O error occurs + */ + public boolean getRevision(File target, String parent, String basename, + String rev) throws IOException { + + Repository repo = getRepository(new File(parent)); + return repo != null && repo.getHistoryGet(target, parent, + basename, rev); + } + /** * Get a named revision of the specified file. * @@ -589,7 +606,8 @@ private void createCache(Repository repository, String sinceRevision) { private void createCacheReal(Collection repositories) { Statistics elapsed = new Statistics(); - ExecutorService executor = RuntimeEnvironment.getHistoryExecutor(); + ExecutorService executor = RuntimeEnvironment.getInstance(). + getIndexerParallelizer().getHistoryExecutor(); // Since we know each repository object from the repositories // collection is unique, we can abuse HashMap to create a list of // repository,revision tuples with repository as key (as the revision @@ -643,25 +661,7 @@ public void run() { } catch (InterruptedException ex) { LOGGER.log(Level.SEVERE, "latch exception{0}", ex); - } - - executor.shutdown(); - while (!executor.isTerminated()) { - try { - // Wait forever - executor.awaitTermination(999, TimeUnit.DAYS); - } catch (InterruptedException exp) { - LOGGER.log(Level.WARNING, - "Received interrupt while waiting for executor to finish", exp); - } - } - RuntimeEnvironment.freeHistoryExecutor(); - try { - /* Thread pool for handling renamed files needs to be destroyed too. */ - RuntimeEnvironment.destroyRenamedHistoryExecutor(); - } catch (InterruptedException ex) { - LOGGER.log(Level.SEVERE, - "destroying of renamed thread pool failed", ex); + return; } // The cache has been populated. Now, optimize how it is stored on diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index d0117547f00..81709b974d6 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2006, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.history; @@ -27,9 +27,11 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.OutputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -41,6 +43,7 @@ import java.util.regex.Pattern; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -206,15 +209,19 @@ Executor getHistoryLogExecutor(File file, String sinceRevision) /** * Try to get file contents for given revision. * + * @param sink a required target sink + * @param outIterations a required out array for storing the number of calls + * made to {@code sink} * @param fullpath full pathname of the file * @param rev revision - * @return contents of the file in revision rev + * @return {@code true} if any contents were found */ - private InputStream getHistoryRev(String fullpath, String rev) { + private boolean getHistoryRev(BufferSink sink, int[] outIterations, + String fullpath, String rev) { InputStream ret = null; + outIterations[0] = 0; File directory = new File(getDirectoryName()); - String revision = rev; if (rev.indexOf(':') != -1) { @@ -229,14 +236,13 @@ private InputStream getHistoryRev(String fullpath, String rev) { RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); int status = executor.exec(); - ByteArrayOutputStream out = new ByteArrayOutputStream(); byte[] buffer = new byte[32 * 1024]; try (InputStream in = executor.getOutputStream()) { int len; - while ((len = in.read(buffer)) != -1) { if (len > 0) { - out.write(buffer, 0, len); + outIterations[0]++; + sink.write(buffer, 0, len); } } } @@ -245,14 +251,42 @@ private InputStream getHistoryRev(String fullpath, String rev) { * If exit value of the process was not 0 then the file did * not exist or internal hg error occured. */ - if (status == 0) { - ret = new ByteArrayInputStream(out.toByteArray()); - } + return status == 0; } catch (Exception exp) { LOGGER.log(Level.SEVERE, "Failed to get history", exp); + return false; } + } - return ret; + /** + * Gets the contents of a specific version of a named file into the + * specified target without a full, in-memory buffer. + * + * @param target a required target file which will be overwritten + * @param parent the name of the directory containing the file + * @param basename the name of the file to get + * @param rev the revision to get + * @return {@code true} if contents were found + * @throws java.io.IOException if an I/O error occurs + */ + @Override + public boolean getHistoryGet(File target, String parent, String basename, + String rev) throws IOException { + try (OutputStream out = new FileOutputStream(target)) { + return getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), + parent, basename, rev); + } + } + + @Override + public InputStream getHistoryGet(String parent, String basename, + String rev) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + if (getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), + parent, basename, rev)) { + return new ByteArrayInputStream(out.toByteArray()); + } + return null; } /** @@ -353,8 +387,8 @@ private String findOriginalName(String fullpath, String full_rev_to_find) return (fullpath.substring(0, getDirectoryName().length() + 1) + file); } - @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { + protected boolean getHistoryGet(BufferSink sink, String parent, + String basename, String rev) { String fullpath; try { @@ -362,11 +396,13 @@ public InputStream getHistoryGet(String parent, String basename, String rev) { } catch (IOException exp) { LOGGER.log(Level.SEVERE, "Failed to get canonical path: {0}", exp.getClass().toString()); - return null; + return false; } - InputStream ret = getHistoryRev(fullpath, rev); - if (ret == null) { + int[] iterations = new int[1]; + boolean ret = getHistoryRev((buf, offset, n) -> sink.write(buf, offset, + n), iterations, fullpath, rev); + if (!ret && iterations[0] < 1) { /* * If we failed to get the contents it might be that the file was * renamed so we need to find its original name in that revision @@ -379,10 +415,10 @@ public InputStream getHistoryGet(String parent, String basename, String rev) { LOGGER.log(Level.SEVERE, "Failed to get original revision: {0}", exp.getClass().toString()); - return null; + return false; } if (origpath != null) { - ret = getHistoryRev(origpath, rev); + ret = getHistoryRev(sink, iterations, origpath, rev); } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java index 619f6aedac9..946f7bc2f15 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java @@ -19,13 +19,15 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.text.DateFormat; import java.text.FieldPosition; import java.text.ParseException; @@ -191,6 +193,36 @@ void removeAndVerifyOldestChangeset(List entries, } } + /** + * Gets the contents of a specific version of a named file into the + * specified target using + * {@link #getHistoryGet(java.lang.String, java.lang.String, java.lang.String)}. + * Subclasses can override to avoid the in-memory copy of the contents. + * + * @param target a required target file which will be overwritten + * @param parent the name of the directory containing the file + * @param basename the name of the file to get + * @param rev the revision to get + * @return {@code true} if contents were found + * @throws java.io.IOException if an I/O error occurs + */ + public boolean getHistoryGet(File target, String parent, String basename, + String rev) throws IOException { + try (InputStream in = getHistoryGet(parent, basename, rev)) { + if (in == null) { + return false; + } + try (OutputStream out = new FileOutputStream(target)) { + byte[] buf = new byte[8 * 1024]; + int n; + while ((n = in.read(buf)) != 0) { + out.write(buf, 0, n); + } + } + return true; + } + } + /** * Get an input stream that I may use to read a specific version of a named * file. diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java index 04d8728471d..f79baf4b69b 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java @@ -45,6 +45,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; @@ -145,7 +146,6 @@ public class IndexDatabase { private List directories; private LockFactory lockfact; private final BytesRef emptyBR = new BytesRef(""); - private IndexerParallelizer parallelizer; // Directory where we store indexes public static final String INDEX_DIR = "index"; @@ -184,23 +184,20 @@ public IndexDatabase(Project project) throws IOException { * Update the index database for all of the projects. Print progress to * standard out. * - * @param parallelizer a defined instance * @throws IOException if an error occurs */ - public static void updateAll(IndexerParallelizer parallelizer) - throws IOException { - updateAll(parallelizer, null); + public static void updateAll() throws IOException { + updateAll(null); } /** * Update the index database for all of the projects * - * @param parallelizer a defined instance * @param listener where to signal the changes to the database * @throws IOException if an error occurs */ - static void updateAll(IndexerParallelizer parallelizer, - IndexChangedListener listener) throws IOException { + static CountDownLatch updateAll(IndexChangedListener listener) + throws IOException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); List dbs = new ArrayList<>(); @@ -212,6 +209,9 @@ static void updateAll(IndexerParallelizer parallelizer, dbs.add(new IndexDatabase()); } + IndexerParallelizer parallelizer = RuntimeEnvironment.getInstance(). + getIndexerParallelizer(); + CountDownLatch latch = new CountDownLatch(dbs.size()); for (IndexDatabase d : dbs) { final IndexDatabase db = d; if (listener != null) { @@ -222,28 +222,31 @@ static void updateAll(IndexerParallelizer parallelizer, @Override public void run() { try { - db.update(parallelizer); + db.update(); } catch (Throwable e) { LOGGER.log(Level.SEVERE, String.format("Problem updating index database in directory %s: ", db.indexDirectory.getDirectory()), e); + } finally { + latch.countDown(); } } }); } + return latch; } /** * Update the index database for a number of sub-directories * - * @param parallelizer a defined instance * @param listener where to signal the changes to the database * @param paths list of paths to be indexed * @throws IOException if an error occurs */ - public static void update(IndexerParallelizer parallelizer, - IndexChangedListener listener, List paths) throws IOException { + public static void update(IndexChangedListener listener, List paths) + throws IOException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + IndexerParallelizer parallelizer = env.getIndexerParallelizer(); List dbs = new ArrayList<>(); for (String path : paths) { @@ -284,7 +287,7 @@ public static void update(IndexerParallelizer parallelizer, @Override public void run() { try { - db.update(parallelizer); + db.update(); } catch (Throwable e) { LOGGER.log(Level.SEVERE, "An error occurred while updating index", e); } @@ -396,11 +399,9 @@ private void markProjectIndexed(Project project) { /** * Update the content of this index database * - * @param parallelizer a defined instance * @throws IOException if an error occurs */ - public void update(IndexerParallelizer parallelizer) - throws IOException { + public void update() throws IOException { synchronized (lock) { if (running) { throw new IOException("Indexer already running!"); @@ -409,7 +410,6 @@ public void update(IndexerParallelizer parallelizer) interrupted = false; } - this.parallelizer = parallelizer; RuntimeEnvironment env = RuntimeEnvironment.getInstance(); reader = null; @@ -572,13 +572,12 @@ public void update(IndexerParallelizer parallelizer) /** * Optimize all index databases * - * @param parallelizer a defined instance * @throws IOException if an error occurs */ - static void optimizeAll(IndexerParallelizer parallelizer) - throws IOException { + static CountDownLatch optimizeAll() throws IOException { List dbs = new ArrayList<>(); RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + IndexerParallelizer parallelizer = env.getIndexerParallelizer(); if (env.hasProjects()) { for (Project project : env.getProjectList()) { dbs.add(new IndexDatabase(project)); @@ -587,6 +586,7 @@ static void optimizeAll(IndexerParallelizer parallelizer) dbs.add(new IndexDatabase()); } + CountDownLatch latch = new CountDownLatch(dbs.size()); for (IndexDatabase d : dbs) { final IndexDatabase db = d; if (db.isDirty()) { @@ -594,15 +594,18 @@ static void optimizeAll(IndexerParallelizer parallelizer) @Override public void run() { try { - db.update(parallelizer); + db.update(); } catch (Throwable e) { LOGGER.log(Level.SEVERE, "Problem updating lucene index database: ", e); + } finally { + latch.countDown(); } } }); } } + return latch; } /** @@ -1173,6 +1176,8 @@ private void indexParallel(IndexDownArgs args) throws IOException { AtomicInteger successCounter = new AtomicInteger(); AtomicInteger currentCounter = new AtomicInteger(); AtomicInteger alreadyClosedCounter = new AtomicInteger(); + IndexerParallelizer parallelizer = RuntimeEnvironment.getInstance(). + getIndexerParallelizer(); ObjectPool ctagsPool = parallelizer.getCtagsPool(); Map> bySuccess = null; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java index 0245f8c5cf5..6dc7ca19ca8 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java @@ -43,6 +43,7 @@ import java.util.Scanner; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -335,6 +336,7 @@ public static void main(String[] argv) { } } + env.getIndexerParallelizer().bounce(); } catch (ParseException e) { System.err.println("** " +e.getMessage()); System.exit(1); @@ -775,6 +777,10 @@ public static String[] parseOptions(String[] argv) throws ParseException { LoggerUtil.setBaseConsoleLogLevel(Level.INFO); }); + parser.on("--webappCtags", "=on|off", ON_OFF, Boolean.class, + "Web application should run ctags when necessary. Default is off."). + Do(v -> cfg.setWebappCtags((Boolean)v)); + parser.on("-W", "--writeConfig", "=/path/to/configuration", "Write the current configuration to the specified file", "(so that the web application can use the same configuration)").Do(configFile -> { @@ -1006,13 +1012,15 @@ public void doIndexerExecution(final boolean update, List subFiles, RuntimeEnvironment env = RuntimeEnvironment.getInstance(); LOGGER.info("Starting indexing"); - IndexerParallelizer parallelizer = new IndexerParallelizer(env); - + IndexerParallelizer parallelizer = env.getIndexerParallelizer(); + final CountDownLatch latch; if (subFiles == null || subFiles.isEmpty()) { if (update) { - IndexDatabase.updateAll(parallelizer, progress); + latch = IndexDatabase.updateAll(progress); } else if (env.isOptimizeDatabase()) { - IndexDatabase.optimizeAll(parallelizer); + latch = IndexDatabase.optimizeAll(); + } else { + latch = new CountDownLatch(0); } } else { List dbs = new ArrayList<>(); @@ -1043,6 +1051,7 @@ public void doIndexerExecution(final boolean update, List subFiles, } } + latch = new CountDownLatch(dbs.size()); for (final IndexDatabase db : dbs) { final boolean optimize = env.isOptimizeDatabase(); db.addIndexChangedListener(progress); @@ -1051,7 +1060,7 @@ public void doIndexerExecution(final boolean update, List subFiles, public void run() { try { if (update) { - db.update(parallelizer); + db.update(); } else if (optimize) { db.optimize(); } @@ -1059,35 +1068,20 @@ public void run() { LOGGER.log(Level.SEVERE, "An error occurred while " + (update ? "updating" : "optimizing") + " index", e); + } finally { + latch.countDown(); } } }); } } - parallelizer.getFixedExecutor().shutdown(); - while (!parallelizer.getFixedExecutor().isTerminated()) { - try { - // Wait forever - parallelizer.getFixedExecutor().awaitTermination(999, - TimeUnit.DAYS); - } catch (InterruptedException exp) { - LOGGER.log(Level.WARNING, "Received interrupt while waiting for executor to finish", exp); - } - } - try { - // It can happen that history index is not done in prepareIndexer() - // but via db.update() above in which case we must make sure the - // thread pool for renamed file handling is destroyed. - RuntimeEnvironment.destroyRenamedHistoryExecutor(); - } catch (InterruptedException ex) { - LOGGER.log(Level.SEVERE, - "destroying of renamed thread pool failed", ex); - } + // Wait forever for the executors to finish. try { - parallelizer.close(); - } catch (Exception ex) { - LOGGER.log(Level.SEVERE, "parallelizer.close() failed", ex); + latch.await(999, TimeUnit.DAYS); + } catch (InterruptedException exp) { + LOGGER.log(Level.WARNING, "Received interrupt while waiting" + + " for executor to finish", exp); } elapsed.report(LOGGER, "Done indexing data of all repositories"); } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexerParallelizer.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexerParallelizer.java index c9424f03ad4..7679f116f28 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexerParallelizer.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexerParallelizer.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2017, Chris Fraire . + * Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.index; @@ -32,6 +32,7 @@ import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; import org.opengrok.indexer.util.BoundedBlockingObjectPool; +import org.opengrok.indexer.util.LazilyInstantiate; import org.opengrok.indexer.util.ObjectFactory; import org.opengrok.indexer.util.ObjectPool; @@ -49,9 +50,23 @@ public class IndexerParallelizer implements AutoCloseable { private static final Logger LOGGER = LoggerFactory.getLogger(IndexerParallelizer.class); - private final ExecutorService fixedExecutor; - private final ForkJoinPool forkJoinPool; - private final ObjectPool ctagsPool; + private final RuntimeEnvironment env; + private final int indexingParallelism; + + private LazilyInstantiate lzForkJoinPool; + private ForkJoinPool forkJoinPool; + + private LazilyInstantiate> lzCtagsPool; + private ObjectPool ctagsPool; + + private LazilyInstantiate lzFixedExecutor; + private ExecutorService fixedExecutor; + + private LazilyInstantiate lzHistoryExecutor; + private ExecutorService historyExecutor; + + private LazilyInstantiate lzHistoryRenamedExecutor; + private ExecutorService historyRenamedExecutor; /** * Initializes a new instance using settings from the specified environment @@ -59,54 +74,158 @@ public class IndexerParallelizer implements AutoCloseable { * @param env a defined instance */ public IndexerParallelizer(RuntimeEnvironment env) { + if (env == null) { + throw new IllegalArgumentException("env is null"); + } + this.env = env; + /* + * Save the following value explicitly because it must not change for + * an IndexerParallelizer instance. + */ + this.indexingParallelism = env.getIndexingParallelism(); - int indexingParallelism = env.getIndexingParallelism(); - - // The order of the following is important. - this.fixedExecutor = Executors.newFixedThreadPool(indexingParallelism); - this.forkJoinPool = new ForkJoinPool(indexingParallelism); - this.ctagsPool = new BoundedBlockingObjectPool<>(indexingParallelism, - new CtagsValidator(), new CtagsObjectFactory(env)); + createLazyForkJoinPool(); + createLazyCtagsPool(); + createLazyFixedExecutor(); + createLazyHistoryExecutor(); + createLazyHistoryRenamedExecutor(); } /** * @return the fixedExecutor */ public ExecutorService getFixedExecutor() { - return fixedExecutor; + ExecutorService result = lzFixedExecutor.get(); + fixedExecutor = result; + return result; } /** * @return the forkJoinPool */ public ForkJoinPool getForkJoinPool() { - return forkJoinPool; + ForkJoinPool result = lzForkJoinPool.get(); + forkJoinPool = result; + return result; } /** * @return the ctagsPool */ public ObjectPool getCtagsPool() { - return ctagsPool; + ObjectPool result = lzCtagsPool.get(); + ctagsPool = result; + return result; } + /** + * @return the ExecutorService used for history parallelism + */ + public ExecutorService getHistoryExecutor() { + ExecutorService result = lzHistoryExecutor.get(); + historyExecutor = result; + return result; + } + + /** + * @return the ExecutorService used for history-renamed parallelism + */ + public ExecutorService getHistoryRenamedExecutor() { + ExecutorService result = lzHistoryRenamedExecutor.get(); + historyRenamedExecutor = result; + return result; + } + + /** + * Calls {@link #bounce()}, which prepares for -- but does not start -- new + * pools. + */ @Override public void close() { - if (ctagsPool != null) { - ctagsPool.shutdown(); + bounce(); + } + + /** + * Shuts down the instance's executors if any of the getters were called; + * and prepares them to be called again to return new instances. + *

+ * N.b. this method is not thread-safe w.r.t. the getters, so care must be + * taken that any scheduled work has been completed and that no other + * thread might call those methods simultaneously with this method. + *

+ * The JVM will await any instantiated thread pools until they are + * explicitly shut down. A principle intention of this method is to + * facilitate OpenGrok test classes that run serially. The non-test + * processes using {@link IndexerParallelizer} -- i.e. {@code opengrok.jar} + * indexer or opengrok-web -- would only need a one-way shutdown; but they + * call this method satisfactorily too. + */ + public void bounce() { + ForkJoinPool formerForkJoinPool = forkJoinPool; + if (formerForkJoinPool != null) { + forkJoinPool = null; + createLazyForkJoinPool(); + formerForkJoinPool.shutdown(); } - if (forkJoinPool != null) { - forkJoinPool.shutdown(); + + ExecutorService formerFixedExecutor = fixedExecutor; + if (formerFixedExecutor != null) { + fixedExecutor = null; + createLazyFixedExecutor(); + formerFixedExecutor.shutdown(); } - if (fixedExecutor != null) { - fixedExecutor.shutdown(); + + ObjectPool formerCtagsPool = ctagsPool; + if (formerCtagsPool != null) { + ctagsPool = null; + createLazyCtagsPool(); + formerCtagsPool.shutdown(); } + + formerFixedExecutor = historyExecutor; + if (formerFixedExecutor != null) { + historyExecutor = null; + createLazyHistoryExecutor(); + formerFixedExecutor.shutdown(); + } + + formerFixedExecutor = historyRenamedExecutor; + if (formerFixedExecutor != null) { + historyRenamedExecutor = null; + createLazyHistoryRenamedExecutor(); + formerFixedExecutor.shutdown(); + } + } + + private void createLazyForkJoinPool() { + lzForkJoinPool = LazilyInstantiate.using(() -> + new ForkJoinPool(indexingParallelism)); + } + + private void createLazyCtagsPool() { + lzCtagsPool = LazilyInstantiate.using(() -> + new BoundedBlockingObjectPool<>(indexingParallelism, + new CtagsValidator(), new CtagsObjectFactory(env))); + } + + private void createLazyFixedExecutor() { + lzFixedExecutor = LazilyInstantiate.using(() -> + Executors.newFixedThreadPool(indexingParallelism)); + } + + private void createLazyHistoryExecutor() { + lzHistoryExecutor = LazilyInstantiate.using(() -> + Executors.newFixedThreadPool(env.getHistoryParallelism())); + } + + private void createLazyHistoryRenamedExecutor() { + lzHistoryRenamedExecutor = LazilyInstantiate.using(() -> + Executors.newFixedThreadPool(env.getHistoryRenamedParallelism())); } /** * Creates a new instance, and attempts to configure it from the specified * environment instance. - * @param env * @return a defined instance, possibly with a {@code null} ctags binary * setting if a value was not available from {@link RuntimeEnvironment}. */ diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/BoundedBlockingObjectPool.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/BoundedBlockingObjectPool.java index 0317bef7473..15db5419c34 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/BoundedBlockingObjectPool.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/BoundedBlockingObjectPool.java @@ -4,16 +4,15 @@ * http://javawithswaranga.blogspot.com/2011/10/generic-and-concurrent-object-pool.html * https://dzone.com/articles/generic-and-concurrent-object : "Feel free to use * it, change it, add more implementations. Happy coding!" - * Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.util; -import java.util.concurrent.BlockingQueue; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -35,11 +34,11 @@ public final class BoundedBlockingObjectPool extends AbstractObjectPool BoundedBlockingObjectPool.class); private final int size; - private final BlockingQueue objects; + private final LinkedBlockingDeque objects; private final ObjectValidator validator; private final ObjectFactory objectFactory; private final ExecutorService executor = Executors.newCachedThreadPool(); - + private volatile boolean puttingLast; private volatile boolean shutdownCalled; public BoundedBlockingObjectPool(int size, ObjectValidator validator, @@ -49,20 +48,27 @@ public BoundedBlockingObjectPool(int size, ObjectValidator validator, this.size = size; this.validator = validator; - objects = new LinkedBlockingQueue<>(size); + objects = new LinkedBlockingDeque<>(size); initializeObjects(); } @Override public T get(long timeOut, TimeUnit unit) { if (!shutdownCalled) { + T ret = null; try { - return objects.poll(timeOut, unit); + ret = objects.pollFirst(timeOut, unit); + /* + * When the queue first empties, switch to a strategy of putting + * returned objects last instead of first. + */ + if (!puttingLast && objects.size() < 1) { + puttingLast = true; + } } catch (InterruptedException ie) { Thread.currentThread().interrupt(); } - - return null; + return ret; } throw new IllegalStateException("Object pool is already shutdown"); } @@ -70,13 +76,20 @@ public T get(long timeOut, TimeUnit unit) { @Override public T get() { if (!shutdownCalled) { + T ret = null; try { - return objects.take(); + ret = objects.takeFirst(); + /* + * When the queue first empties, switch to a strategy of putting + * returned objects last instead of first. + */ + if (!puttingLast && objects.size() < 1) { + puttingLast = true; + } } catch (InterruptedException ie) { Thread.currentThread().interrupt(); } - - return null; + return ret; } throw new IllegalStateException("Object pool is already shutdown"); } @@ -97,7 +110,7 @@ private void clearResources() { @Override protected void returnToPool(T t) { if (validator.isValid(t)) { - executor.submit(new ObjectReturner(objects, t)); + executor.submit(new ObjectReturner<>(objects, t, puttingLast)); } } @@ -112,7 +125,7 @@ protected void handleInvalidReturn(T t) { } t = objectFactory.createNew(); - executor.submit(new ObjectReturner(objects, t)); + executor.submit(new ObjectReturner<>(objects, t, puttingLast)); } @Override @@ -127,19 +140,25 @@ private void initializeObjects() { } private class ObjectReturner implements Callable { - private final BlockingQueue queue; + private final LinkedBlockingDeque queue; private final E e; + private final boolean puttingLast; - ObjectReturner(BlockingQueue queue, E e) { + ObjectReturner(LinkedBlockingDeque queue, E e, boolean puttingLast) { this.queue = queue; this.e = e; + this.puttingLast = puttingLast; } @Override public Void call() { while (true) { try { - queue.put(e); + if (puttingLast) { + queue.putLast(e); + } else { + queue.putFirst(e); + } break; } catch (InterruptedException ie) { Thread.currentThread().interrupt(); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/BufferSink.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/BufferSink.java new file mode 100644 index 00000000000..2920d865358 --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/BufferSink.java @@ -0,0 +1,34 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2018, Chris Fraire . + */ + +package org.opengrok.indexer.util; + +import java.io.IOException; + +/** + * Represents a functional interface for accepting buffers. + */ +@FunctionalInterface +public interface BufferSink { + void write(byte[] buf, int offset, int n) throws IOException; +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/LazilyInstantiate.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/LazilyInstantiate.java new file mode 100644 index 00000000000..c11588d810d --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/LazilyInstantiate.java @@ -0,0 +1,111 @@ +/* + * The contents of this file are Copyright (c) 2014, + * https://programmingideaswithjake.wordpress.com/about/ made available under + * free license, + * https://programmingideaswithjake.wordpress.com/2014/10/05/java-functional-lazy-instantiation/ + * https://github.com/EZGames/functional-java/blob/master/LICENSE : + * "There is no license on this code. + * It's true open-ware. Do what you want with it. + * It's only meant to be helpful + * I'd prefer that you don't take credit for it, though. You don't have to give + * credit; just don't take it." + * Portions Copyright (c) 2018, Chris Fraire . + */ +package org.opengrok.indexer.util; + +import java.util.function.Supplier; + +/** + * {@code LazilyInstantiate} is a quick class to make lazily instantiating + * objects easy. All you need to know for working with this class is its two + * public methods: one static factory for creating the object, and another for + * initiating the instantiation and retrieval of the object. + *

+ * Also, another benefit is that it's thread safe, but only blocks when + * initially instantiating the object. After that, it stops blocking and removes + * unnecessary checks for whether the object is instantiated. + *

+ * Here's an example of it being used for implementing a singleton: + * + * public class Singleton
+ * {
+ *     private static Supplier<Singleton> instance = + * LazilyInstantiate.using(() -> new Singleton());
+ *     //other fields
+ *
+ *     public static getInstance()
+ *     {
+ *         instance.get();
+ *     }
+ *
+ *     //other methods
+ *
+ *     private Singleton()
+ *     {
+ *         //contructor stuff
+ *     }
+ * }
+ * + *

+ * So, here are the changes you'll need to apply in your code: + *

    + *
  • Change the type of the lazily instantiated object to a {@code Supplier} + * of that type + *
  • Have it set to LazilyInstantiate.using() where the argument is + * {@code () -> } You could also use a method reference, + * which, for the example above, would be {@code Singleton::new} instead of + * {@code () -> new Singleton()}
    • + *
  • Whatever asks for the object, asks for the {@code Supplier} object, then + * {@code .get()}
    • + *
+ * + * @param the type of object that you're trying to lazily instantiate + */ +public class LazilyInstantiate implements Supplier { + + private final Supplier supplier; + private Supplier current; + + public static LazilyInstantiate using(Supplier supplier) { + return new LazilyInstantiate<>(supplier); + } + + /** + * Executes the {@link #using(java.util.function.Supplier)} supplier in a + * thread-safe manner if it has not yet been executed, and keeps the result + * to provide to every caller of this method. + * @return the result of {@code supplier} + */ + @Override + public T get() { + return current.get(); + } + + private LazilyInstantiate(Supplier supplier) { + this.supplier = supplier; + this.current = () -> swapper(); + } + + //swaps the itself out for a supplier of an instantiated object + private synchronized T swapper() { + if (!Factory.class.isInstance(current)) { + T obj = supplier.get(); + current = new Factory<>(obj); + } + return current.get(); + } + + private class Factory implements Supplier { + + private final T obj; + + Factory(T obj) { + this.obj = obj; + } + + @Override + public T get() { + return obj; + } + } +} diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java index bc0a2fa7881..19189f1c735 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java @@ -60,7 +60,6 @@ public class IndexDatabaseTest { private static TestRepository repository; - private static IndexerParallelizer parallelizer; @ClassRule public static ConditionalRunRule rule = new ConditionalRunRule(); @@ -79,8 +78,6 @@ public static void setUpClass() throws Exception { env.setProjectsEnabled(true); RepositoryFactory.initializeIgnoredNames(env); - parallelizer = new IndexerParallelizer(env); - // Note that all tests in this class share the index created below. // Ergo, if they need to modify it, this has to be done in such a way // so that it does not affect other tests, no matter in which order @@ -96,11 +93,6 @@ public static void setUpClass() throws Exception { @AfterClass public static void tearDownClass() throws Exception { repository.destroy(); - - if (parallelizer != null) { - parallelizer.close(); - parallelizer = null; - } } @Test @@ -179,7 +171,7 @@ public void testCleanupAfterIndexRemoval() throws Exception { File file = new File(repository.getSourceRoot(), projectName + File.separator + fileName); file.delete(); Assert.assertFalse("file " + fileName + " not removed", file.exists()); - idb.update(parallelizer); + idb.update(); // Check that the data for the file has been removed. checkDataExistence(projectName + File.separator + fileName, false); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java index 57bf69097a0..d62fa5a91da 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java @@ -77,7 +77,6 @@ public class IndexerTest { TestRepository repository; - private static IndexerParallelizer parallelizer; @Rule public ConditionalRunRule rule = new ConditionalRunRule(); @@ -86,16 +85,6 @@ public class IndexerTest { public static void setUpClass() { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); RepositoryFactory.initializeIgnoredNames(env); - - parallelizer = new IndexerParallelizer(env); - } - - @AfterClass - public static void tearDownClass() throws Exception { - if (parallelizer != null) { - parallelizer.close(); - parallelizer = null; - } } @Before @@ -257,14 +246,14 @@ public void testIndexWithSetIndexVersionedFilesOnly() throws Exception { assertNotNull(idb); MyIndexChangeListener listener = new MyIndexChangeListener(); idb.addIndexChangedListener(listener); - idb.update(parallelizer); + idb.update(); assertEquals(2, listener.files.size()); repository.purgeData(); RuntimeEnvironment.getInstance().setIndexVersionedFilesOnly(true); idb = new IndexDatabase(project); listener = new MyIndexChangeListener(); idb.addIndexChangedListener(listener); - idb.update(parallelizer); + idb.update(); assertEquals(1, listener.files.size()); RuntimeEnvironment.getInstance().setIndexVersionedFilesOnly(false); } else { @@ -340,7 +329,7 @@ public void testRemoveFileOnFileChange() throws Exception { assertNotNull(idb); RemoveIndexChangeListener listener = new RemoveIndexChangeListener(); idb.addIndexChangedListener(listener); - idb.update(parallelizer); + idb.update(); Assert.assertEquals(5, listener.filesToAdd.size()); listener.reset(); @@ -356,7 +345,7 @@ public void testRemoveFileOnFileChange() throws Exception { fw.close(); // reindex - idb.update(parallelizer); + idb.update(); // Make sure that the file was actually processed. assertEquals(1, listener.removedFiles.size()); assertEquals(1, listener.filesToAdd.size()); @@ -396,7 +385,7 @@ public void testBug3430() throws Exception { assertNotNull(idb); MyIndexChangeListener listener = new MyIndexChangeListener(); idb.addIndexChangedListener(listener); - idb.update(parallelizer); + idb.update(); assertEquals(1, listener.files.size()); } @@ -416,14 +405,14 @@ public void testIncrementalIndexAddRemoveFile() throws Exception { assertNotNull(idb); MyIndexChangeListener listener = new MyIndexChangeListener(); idb.addIndexChangedListener(listener); - idb.update(parallelizer); + idb.update(); assertEquals(1, listener.files.size()); listener.reset(); repository.addDummyFile(ppath); - idb.update(parallelizer); + idb.update(); assertEquals("No new file added", 1, listener.files.size()); repository.removeDummyFile(ppath); - idb.update(parallelizer); + idb.update(); assertEquals("(added)files changed unexpectedly", 1, listener.files.size()); assertEquals("Didn't remove the dummy file", 1, listener.removedFiles.size()); assertEquals("Should have added then removed the same file", @@ -462,7 +451,7 @@ public void testBug11896() throws Exception { MyIndexChangeListener listener = new MyIndexChangeListener(); idb.addIndexChangedListener(listener); System.out.println("Trying to index a special file - FIFO in this case."); - idb.update(parallelizer); + idb.update(); assertEquals(0, listener.files.size()); } else { diff --git a/opengrok-web/src/main/java/org/opengrok/web/WebappListener.java b/opengrok-web/src/main/java/org/opengrok/web/WebappListener.java index 38291e73a7e..39714e532b6 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/WebappListener.java +++ b/opengrok-web/src/main/java/org/opengrok/web/WebappListener.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.web; @@ -109,8 +110,10 @@ public void contextInitialized(final ServletContextEvent servletContextEvent) { */ @Override public void contextDestroyed(final ServletContextEvent servletContextEvent) { - RuntimeEnvironment.getInstance().watchDog.stop(); - RuntimeEnvironment.getInstance().stopExpirationTimer(); + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + env.getIndexerParallelizer().bounce(); + env.watchDog.stop(); + env.stopExpirationTimer(); try { saveStatistics(); } catch (IOException ex) { diff --git a/opengrok-web/src/main/webapp/list.jsp b/opengrok-web/src/main/webapp/list.jsp index 07ff6c452ec..ac1ab9515a9 100644 --- a/opengrok-web/src/main/webapp/list.jsp +++ b/opengrok-web/src/main/webapp/list.jsp @@ -33,21 +33,27 @@ java.net.URLEncoder, java.nio.charset.StandardCharsets, java.util.List, java.util.Locale, +java.util.logging.Level, java.util.Set, +java.util.logging.Logger, org.opengrok.indexer.analysis.AnalyzerGuru, +org.opengrok.indexer.analysis.Ctags, org.opengrok.indexer.analysis.Definitions, +org.opengrok.indexer.analysis.AbstractAnalyzer, +org.opengrok.indexer.analysis.AbstractAnalyzer.Genre, +org.opengrok.indexer.analysis.AnalyzerFactory, org.opengrok.indexer.history.Annotation, org.opengrok.indexer.index.IndexDatabase, +org.opengrok.indexer.logger.LoggerFactory, org.opengrok.indexer.search.DirectoryEntry, org.opengrok.indexer.search.DirectoryExtraReader, org.opengrok.indexer.search.FileExtra, org.opengrok.indexer.util.FileExtraZipper, +org.opengrok.indexer.util.ObjectPool, org.opengrok.indexer.util.IOUtils, org.opengrok.web.DirectoryListing, org.opengrok.indexer.web.SearchHelper" %> -<%@ page import="org.opengrok.indexer.analysis.AnalyzerFactory" %> -<%@ page import="org.opengrok.indexer.analysis.AbstractAnalyzer" %> <% { // need to set it here since requesting parameters @@ -78,6 +84,8 @@ document.pageReady.push(function() { pageReadyList();}); <% /* ---------------------- list.jsp start --------------------- */ { + final Logger LOGGER = LoggerFactory.getLogger(getClass()); + PageConfig cfg = PageConfig.get(request); String rev = cfg.getRequestedRevision(); Project project = cfg.getProject(); @@ -240,25 +248,37 @@ Click download <%= basename %><% } else { // requesting a previous revision or needed to generate xref on the fly (economy mode). AnalyzerFactory a = AnalyzerGuru.find(basename); - AbstractAnalyzer.Genre g = AnalyzerGuru.getGenre(a); + Genre g = AnalyzerGuru.getGenre(a); String error = null; - if (g == AbstractAnalyzer.Genre.PLAIN || g == AbstractAnalyzer.Genre.HTML || g == null) { + if (g == Genre.PLAIN || g == Genre.HTML || g == null) { InputStream in = null; + File tempf = null; try { if (rev.equals(DUMMY_REVISION)) { in = new FileInputStream(resourceFile); } else { - in = HistoryGuru.getInstance() - .getRevision(resourceFile.getParent(), basename, rev); + tempf = File.createTempFile("ogtags", basename); + if (HistoryGuru.getInstance().getRevision(tempf, + resourceFile.getParent(), basename, rev)) { + in = new BufferedInputStream( + new FileInputStream(tempf)); + } else { + tempf.delete(); + tempf = null; + } } } catch (Exception e) { // fall through to error message error = e.getMessage(); + if (tempf != null) { + tempf.delete(); + tempf = null; + } } if (in != null) { try { if (g == null) { - a = AnalyzerGuru.find(in); + a = AnalyzerGuru.find(in, basename); g = AnalyzerGuru.getGenre(a); } if (g == AbstractAnalyzer.Genre.DATA || g == AbstractAnalyzer.Genre.XREFABLE || g == null) { @@ -271,9 +291,34 @@ Click download <%= basename %><% 
<%
                             if (g == AbstractAnalyzer.Genre.PLAIN) {
-                                // We don't have any way to get definitions
-                                // for old revisions currently.
                                 Definitions defs = null;
+                                ObjectPool ctagsPool = cfg.getEnv().
+                                        getIndexerParallelizer().getCtagsPool();
+                                int tries = 2;
+                                while (cfg.getEnv().isWebappCtags()) {
+                                    Ctags ctags = ctagsPool.get();
+                                    try {
+                                        ctags.setTabSize(project != null ?
+                                                project.getTabSize() : 0);
+                                        defs = ctags.doCtags(tempf.getPath());
+                                        break;
+                                    } catch (InterruptedException ex) {
+                                        if (--tries > 0) {
+                                            LOGGER.log(Level.WARNING,
+                                                    "doCtags() interrupted--{0}",
+                                                    ex.getMessage());
+                                            continue;
+                                        }
+                                        LOGGER.log(Level.WARNING, "doCtags()", ex);
+                                        break;
+                                    } catch (Exception ex) {
+                                        LOGGER.log(Level.WARNING, "doCtags()", ex);
+                                        break;
+                                    } finally {
+                                        ctags.reset();
+                                        ctagsPool.release(ctags);
+                                    }
+                                }
                                 Annotation annotation = cfg.getAnnotation();
                                 //not needed yet
                                 //annotation.writeTooltipMap(out);
@@ -315,6 +360,9 @@ Click download <%= basename %><%
                             try { in.close(); }
                             catch (Exception e) { /* ignore */ }
                         }
+                        if (tempf != null) {
+                            tempf.delete();
+                        }
                     }
         %>
<% From 2049f2bc88e2beb07a229ed0b0a875a31368642e Mon Sep 17 00:00:00 2001 From: Chris Fraire Date: Sat, 8 Dec 2018 16:41:00 -0600 Subject: [PATCH 2/4] Use object for multi-return value. Fix value check. --- .../indexer/history/GitRepository.java | 38 ++++++----------- .../indexer/history/MercurialRepository.java | 41 +++++++------------ .../opengrok/indexer/history/Repository.java | 28 ++++++++++++- 3 files changed, 54 insertions(+), 53 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java index bd75f752c3f..fe3ce11b5bc 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java @@ -175,16 +175,16 @@ Executor getRenamedFilesExecutor(final File file, String sinceRevision) throws I * Try to get file contents for given revision. * * @param sink a required target sink - * @param outIterations a required out array for storing the number of calls - * made to {@code sink} * @param fullpath full pathname of the file * @param rev revision - * @return {@code true} if any contents were found + * @return a defined instance with {@code success} == {@code true} if no + * error occurred and with non-zero {@code iterations} if some data was + * transferred */ - private boolean getHistoryRev(BufferSink sink, int[] outIterations, - String fullpath, String rev) { + private HistoryRevResult getHistoryRev( + BufferSink sink, String fullpath, String rev) { - outIterations[0] = 0; + HistoryRevResult result = new HistoryRevResult(); File directory = new File(getDirectoryName()); try { /* @@ -203,29 +203,19 @@ private boolean getHistoryRev(BufferSink sink, int[] outIterations, Executor executor = new Executor(Arrays.asList(argv), directory, RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); int status = executor.exec(); - - byte[] buffer = new byte[32 * 1024]; - try (InputStream in = executor.getOutputStream()) { - int len; - while ((len = in.read(buffer)) != -1) { - if (len > 0) { - outIterations[0]++; - sink.write(buffer, 0, len); - } - } - } + result.iterations = transferExecutorOutput(sink, executor); /* * If exit value of the process was not 0 then the file did * not exist or internal git error occured. */ - return status == 0; + result.success = (status == 0); } catch (Exception exp) { LOGGER.log(Level.SEVERE, "Failed to get history for file {0} in revision {1}: ", new Object[]{fullpath, rev, exp.getClass().toString(), exp}); - return false; } + return result; } /** @@ -274,10 +264,8 @@ public String get() { return false; } - int[] iterations = new int[1]; - boolean ret = getHistoryRev((buf, offset, n) -> sink.write(buf, offset, - n), iterations, fullpath, rev); - if (!ret && iterations[0] < 1) { + HistoryRevResult result = getHistoryRev(sink::write, fullpath, rev); + if (!result.success && result.iterations < 1) { /* * If we failed to get the contents it might be that the file was * renamed so we need to find its original name in that revision @@ -296,11 +284,11 @@ public String get() { return false; } if (origpath != null) { - ret = getHistoryRev(sink, iterations, origpath, rev); + result = getHistoryRev(sink, origpath, rev); } } - return ret; + return result.success; } /** diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index 81709b974d6..fd6bcb7cc4b 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -210,20 +210,19 @@ Executor getHistoryLogExecutor(File file, String sinceRevision) * Try to get file contents for given revision. * * @param sink a required target sink - * @param outIterations a required out array for storing the number of calls - * made to {@code sink} * @param fullpath full pathname of the file * @param rev revision - * @return {@code true} if any contents were found + * @return a defined instance with {@code success} == {@code true} if no + * error occurred and with non-zero {@code iterations} if some data was + * transferred */ - private boolean getHistoryRev(BufferSink sink, int[] outIterations, - String fullpath, String rev) { - InputStream ret = null; + private HistoryRevResult getHistoryRev( + BufferSink sink, String fullpath, String rev) { - outIterations[0] = 0; + HistoryRevResult result = new HistoryRevResult(); File directory = new File(getDirectoryName()); - String revision = rev; + String revision = rev; if (rev.indexOf(':') != -1) { revision = rev.substring(0, rev.indexOf(':')); } @@ -235,27 +234,17 @@ private boolean getHistoryRev(BufferSink sink, int[] outIterations, Executor executor = new Executor(Arrays.asList(argv), directory, RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); int status = executor.exec(); - - byte[] buffer = new byte[32 * 1024]; - try (InputStream in = executor.getOutputStream()) { - int len; - while ((len = in.read(buffer)) != -1) { - if (len > 0) { - outIterations[0]++; - sink.write(buffer, 0, len); - } - } - } + result.iterations = transferExecutorOutput(sink, executor); /* * If exit value of the process was not 0 then the file did * not exist or internal hg error occured. */ - return status == 0; + result.success = (status == 0); } catch (Exception exp) { LOGGER.log(Level.SEVERE, "Failed to get history", exp); - return false; } + return result; } /** @@ -399,10 +388,8 @@ protected boolean getHistoryGet(BufferSink sink, String parent, return false; } - int[] iterations = new int[1]; - boolean ret = getHistoryRev((buf, offset, n) -> sink.write(buf, offset, - n), iterations, fullpath, rev); - if (!ret && iterations[0] < 1) { + HistoryRevResult result = getHistoryRev(sink::write, fullpath, rev); + if (!result.success && result.iterations < 1) { /* * If we failed to get the contents it might be that the file was * renamed so we need to find its original name in that revision @@ -418,11 +405,11 @@ protected boolean getHistoryGet(BufferSink sink, String parent, return false; } if (origpath != null) { - ret = getHistoryRev(sink, iterations, origpath, rev); + result = getHistoryRev(sink, origpath, rev); } } - return ret; + return result.success; } /** diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java index 946f7bc2f15..6b320c15017 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java @@ -43,6 +43,7 @@ import java.util.logging.Logger; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -215,7 +216,7 @@ public boolean getHistoryGet(File target, String parent, String basename, try (OutputStream out = new FileOutputStream(target)) { byte[] buf = new byte[8 * 1024]; int n; - while ((n = in.read(buf)) != 0) { + while ((n = in.read(buf)) != -1) { out.write(buf, 0, n); } } @@ -582,6 +583,31 @@ protected String getRepoRelativePath(final File file) return filename; } + /** + * Transfers the {@code executor} output to the {@code sink}. + * @return the number of calls to {@code sink} + */ + static int transferExecutorOutput(BufferSink sink, Executor executor) + throws IOException { + try (InputStream in = executor.getOutputStream()) { + byte[] buffer = new byte[8 * 1024]; + int iterations = 0; + int len; + while ((len = in.read(buffer)) != -1) { + if (len > 0) { + ++iterations; + sink.write(buffer, 0, len); + } + } + return iterations; + } + } + + class HistoryRevResult { + public boolean success; + public int iterations; + } + private class RepositoryDateFormat extends DateFormat { private static final long serialVersionUID = -6951382723884436414L; From 6e090195c321a96116d723f41b8c5e992f514de2 Mon Sep 17 00:00:00 2001 From: Chris Fraire Date: Sun, 9 Dec 2018 13:50:32 -0600 Subject: [PATCH 3/4] Use BufferSink generally in Repositories. Remove redundancies. --- .../indexer/history/AccuRevRepository.java | 20 +++-- .../indexer/history/BazaarRepository.java | 27 ++----- .../indexer/history/BitKeeperRepository.java | 33 +++++--- .../indexer/history/ClearCaseRepository.java | 35 ++++---- .../indexer/history/GitRepository.java | 41 +--------- .../indexer/history/MercurialRepository.java | 47 ++--------- .../indexer/history/MonotoneRepository.java | 34 +++----- .../indexer/history/PerforceRepository.java | 22 +++-- .../indexer/history/RCSRepository.java | 12 ++- .../indexer/history/RazorRepository.java | 36 +++++---- .../indexer/history/RepoRepository.java | 9 ++- .../opengrok/indexer/history/Repository.java | 81 ++++++++++--------- .../indexer/history/SCCSRepository.java | 14 +++- .../indexer/history/SSCMRepository.java | 48 ++++++----- .../indexer/history/SubversionRepository.java | 20 +++-- .../indexer/history/RepositoryTest.java | 9 ++- 16 files changed, 222 insertions(+), 266 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java index 4026b3af4be..1d542036d77 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java @@ -19,14 +19,13 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.BufferedReader; -import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Paths; import java.nio.file.Path; @@ -38,6 +37,7 @@ import java.util.regex.Pattern; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -165,10 +165,10 @@ Executor getHistoryLogExecutor(File file) throws IOException { } @Override - InputStream getHistoryGet(String parent, String basename, String rev) { + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { ArrayList cmd = new ArrayList<>(); - InputStream inputStream = null; File directory = new File(parent); /* @@ -216,12 +216,16 @@ InputStream getHistoryGet(String parent, String basename, String rev) { executor = new Executor(cmd, directory); executor.exec(); - - inputStream - = new ByteArrayInputStream(executor.getOutputString().getBytes()); + try { + copyBytes(sink, executor.getOutputStream()); + return true; + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Failed to obtain content for {0}", + basename); + } } - return inputStream; + return false; } @Override diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java index 4294dc14646..66469579c9a 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java @@ -19,12 +19,10 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.history; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -35,6 +33,7 @@ import java.util.logging.Logger; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -96,11 +95,10 @@ Executor getHistoryLogExecutor(final File file, final String sinceRevision) } @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { - InputStream ret = null; + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { File directory = new File(getDirectoryName()); - Process process = null; try { String filename = (new File(parent, basename)).getCanonicalPath() @@ -108,19 +106,10 @@ public InputStream getHistoryGet(String parent, String basename, String rev) { ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); String[] argv = {RepoCommand, "cat", "-r", rev, filename}; process = Runtime.getRuntime().exec(argv, null, directory); - - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buffer = new byte[32 * 1024]; - InputStream in = process.getInputStream(); - int len; - - while ((len = in.read(buffer)) != -1) { - if (len > 0) { - out.write(buffer, 0, len); - } + try (InputStream in = process.getInputStream()) { + copyBytes(sink, in); } - - ret = new ByteArrayInputStream(out.toByteArray()); + return true; } catch (Exception exp) { LOGGER.log(Level.SEVERE, "Failed to get history: " + exp.getClass().toString(), exp); @@ -136,7 +125,7 @@ public InputStream getHistoryGet(String parent, String basename, String rev) { } } - return ret; + return false; } /** diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java index 5970cd09f3f..725a9b5dd8a 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java @@ -17,17 +17,23 @@ * CDDL HEADER END */ +/* + * Author: James Service + * Portions by: Oracle and/or its affiliates. + * Portions Copyright (c) 2018, Chris Fraire . + */ + package org.opengrok.indexer.history; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.opengrok.indexer.util.BufferSink; import org.suigeneris.jrcs.rcs.InvalidVersionNumberException; import org.suigeneris.jrcs.rcs.Version; import org.opengrok.indexer.configuration.RuntimeEnvironment; @@ -294,18 +300,11 @@ History getHistory(File file, String sinceRevision) throws HistoryException { return history; } - /** - * Return an InputStream of the content of a given file at a given revision. - * - * @param parent the directory the file is in - * @param basename the basename of the file - * @param revision revision, or null for latest - * @return output an input stream - */ @Override - public InputStream getHistoryGet(String parent, String basename, String revision) { - final File directory = new File(parent).getAbsoluteFile(); + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String revision) { + final File directory = new File(parent).getAbsoluteFile(); final ArrayList argv = new ArrayList(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); @@ -320,10 +319,18 @@ public InputStream getHistoryGet(String parent, String basename, String revision RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); if (executor.exec(true) != 0) { LOGGER.log(Level.SEVERE, "Failed to get history: {0}", executor.getErrorString()); - return null; + return false; } - return executor.getOutputStream(); + try { + copyBytes(sink, executor.getOutputStream()); + return true; + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Failed to get content for {0}", + basename); + } + + return false; } /* Annotation Stuff */ diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java index 86e5d2a4e73..91531ed739c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java @@ -19,16 +19,14 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.history; -import java.io.BufferedInputStream; import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; import java.io.IOException; -import java.io.InputStream; import java.io.InputStreamReader; import java.util.ArrayList; import java.util.Arrays; @@ -38,6 +36,7 @@ import java.util.regex.Pattern; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -91,8 +90,8 @@ Executor getHistoryLogExecutor(final File file) throws IOException { } @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { - InputStream ret = null; + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { File directory = new File(getDirectoryName()); @@ -117,28 +116,26 @@ public InputStream getHistoryGet(String parent, String basename, String rev) { if (status != 0) { LOGGER.log(Level.SEVERE, "Failed to get history: {0}", executor.getErrorString()); - return null; + return false; } - ret = new BufferedInputStream(new FileInputStream(tmp)) { - - @Override - public void close() throws IOException { - super.close(); - // delete the temporary file on close - if (!tmp.delete()) { - // failed, lets do the next best thing then .. - // delete it on JVM exit - tmp.deleteOnExit(); - } + try (FileInputStream in = new FileInputStream(tmp)) { + copyBytes(sink, in); + } finally { + // delete the temporary file on close + if (!tmp.delete()) { + // failed, lets do the next best thing then .. + // delete it on JVM exit + tmp.deleteOnExit(); } - }; + } + return true; } catch (Exception exp) { LOGGER.log(Level.WARNING, "Failed to get history: " + exp.getClass().toString(), exp); } - return ret; + return false; } /** diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java index fe3ce11b5bc..35ca1b76dd4 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java @@ -24,14 +24,10 @@ package org.opengrok.indexer.history; import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.io.OutputStream; import java.io.Reader; import java.nio.file.Paths; import java.text.ParseException; @@ -203,7 +199,7 @@ private HistoryRevResult getHistoryRev( Executor executor = new Executor(Arrays.asList(argv), directory, RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); int status = executor.exec(); - result.iterations = transferExecutorOutput(sink, executor); + result.iterations = copyBytes(sink, executor.getOutputStream()); /* * If exit value of the process was not 0 then the file did @@ -218,39 +214,10 @@ private HistoryRevResult getHistoryRev( return result; } - /** - * Gets the contents of a specific version of a named file into the - * specified target without a full, in-memory buffer. - * - * @param target a required target file which will be overwritten - * @param parent the name of the directory containing the file - * @param basename the name of the file to get - * @param rev the revision to get - * @return {@code true} if contents were found - * @throws java.io.IOException if an I/O error occurs - */ - @Override - public boolean getHistoryGet(File target, String parent, String basename, - String rev) throws IOException { - try (OutputStream out = new FileOutputStream(target)) { - return getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), - parent, basename, rev); - } - } - @Override - public InputStream getHistoryGet(String parent, String basename, - String rev) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - if (getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), - parent, basename, rev)) { - return new ByteArrayInputStream(out.toByteArray()); - } - return null; - } + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { - protected boolean getHistoryGet(BufferSink sink, String parent, - String basename, String rev) { String fullpath; try { fullpath = new File(parent, basename).getCanonicalPath(); @@ -283,7 +250,7 @@ public String get() { }); return false; } - if (origpath != null) { + if (origpath != null && !origpath.equals(fullpath)) { result = getHistoryRev(sink, origpath, rev); } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index fd6bcb7cc4b..62d81fc9c44 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -24,14 +24,9 @@ package org.opengrok.indexer.history; import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStream; import java.io.InputStreamReader; -import java.io.OutputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -234,7 +229,7 @@ private HistoryRevResult getHistoryRev( Executor executor = new Executor(Arrays.asList(argv), directory, RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); int status = executor.exec(); - result.iterations = transferExecutorOutput(sink, executor); + result.iterations = copyBytes(sink, executor.getOutputStream()); /* * If exit value of the process was not 0 then the file did @@ -247,37 +242,6 @@ private HistoryRevResult getHistoryRev( return result; } - /** - * Gets the contents of a specific version of a named file into the - * specified target without a full, in-memory buffer. - * - * @param target a required target file which will be overwritten - * @param parent the name of the directory containing the file - * @param basename the name of the file to get - * @param rev the revision to get - * @return {@code true} if contents were found - * @throws java.io.IOException if an I/O error occurs - */ - @Override - public boolean getHistoryGet(File target, String parent, String basename, - String rev) throws IOException { - try (OutputStream out = new FileOutputStream(target)) { - return getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), - parent, basename, rev); - } - } - - @Override - public InputStream getHistoryGet(String parent, String basename, - String rev) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - if (getHistoryGet((buf, offset, n) -> out.write(buf, offset, n), - parent, basename, rev)) { - return new ByteArrayInputStream(out.toByteArray()); - } - return null; - } - /** * Get the name of file in given revision. This is used to get contents * of a file in historical revision. @@ -376,10 +340,11 @@ private String findOriginalName(String fullpath, String full_rev_to_find) return (fullpath.substring(0, getDirectoryName().length() + 1) + file); } - protected boolean getHistoryGet(BufferSink sink, String parent, - String basename, String rev) { - String fullpath; + @Override + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { + String fullpath; try { fullpath = new File(parent, basename).getCanonicalPath(); } catch (IOException exp) { @@ -404,7 +369,7 @@ protected boolean getHistoryGet(BufferSink sink, String parent, exp.getClass().toString()); return false; } - if (origpath != null) { + if (origpath != null && !origpath.equals(fullpath)) { result = getHistoryRev(sink, origpath, rev); } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java index 82719cc39ce..583f68ee01b 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java @@ -19,17 +19,13 @@ /* * Copyright (c) 2009, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.history; -import java.io.BufferedInputStream; import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -37,6 +33,7 @@ import java.util.logging.Logger; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -67,38 +64,25 @@ public MonotoneRepository() { } @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { - InputStream ret = null; - File directory = new File(getDirectoryName()); - String revision = rev; + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { + File directory = new File(getDirectoryName()); try { String filename = (new File(parent, basename)).getCanonicalPath() .substring(getDirectoryName().length() + 1); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); - String[] argv = {RepoCommand, "cat", "-r", revision, filename}; + String[] argv = {RepoCommand, "cat", "-r", rev, filename}; Executor executor = new Executor(Arrays.asList(argv), directory, RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); - - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buffer = new byte[32 * 1024]; - try (InputStream in = executor.getOutputStream()) { - int len; - - while ((len = in.read(buffer)) != -1) { - if (len > 0) { - out.write(buffer, 0, len); - } - } - } - - ret = new BufferedInputStream(new ByteArrayInputStream(out.toByteArray())); + copyBytes(sink, executor.getOutputStream()); + return true; } catch (Exception exp) { LOGGER.log(Level.SEVERE, "Failed to get history: {0}", exp.getClass().toString()); } - return ret; + return false; } /** diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java index c4e54ab9893..adc352e8043 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java @@ -19,19 +19,20 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.indexer.history; -import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.List; +import java.util.logging.Level; import java.util.logging.Logger; -import org.opengrok.indexer.configuration.RuntimeEnvironment; +import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -78,7 +79,9 @@ public Annotation annotate(File file, String rev) throws IOException { } @Override - InputStream getHistoryGet(String parent, String basename, String rev) { + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { + ArrayList cmd = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); cmd.add(RepoCommand); @@ -86,8 +89,17 @@ InputStream getHistoryGet(String parent, String basename, String rev) { cmd.add("-q"); cmd.add(basename + getRevisionCmd(rev)); Executor executor = new Executor(cmd, new File(parent)); + // TODO: properly evaluate Perforce return code executor.exec(); - return new ByteArrayInputStream(executor.getOutputString().getBytes()); + try { + copyBytes(sink, executor.getOutputStream()); + return true; + } catch (IOException e) { + LOGGER.log(Level.SEVERE, + "Failed to get history for file {0}/{1} in revision {2}: ", + new Object[]{parent, basename, rev}); + } + return false; } @Override diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java index c40733076dd..5efb45a55d3 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.indexer.history; @@ -33,6 +34,7 @@ import java.util.logging.Logger; import org.opengrok.indexer.configuration.RuntimeEnvironment; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; import org.opengrok.indexer.logger.LoggerFactory; @@ -68,15 +70,19 @@ boolean fileHasHistory(File file) { } @Override - InputStream getHistoryGet(String parent, String basename, String rev) { + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { try { File file = new File(parent, basename); File rcsFile = getRCSFile(file); - return new RCSget(rcsFile.getPath(), rev); + try (InputStream in = new RCSget(rcsFile.getPath(), rev)) { + copyBytes(sink, in); + } + return true; } catch (IOException ioe) { LOGGER.log(Level.SEVERE, "Failed to retrieve revision " + rev + " of " + basename, ioe); - return null; + return false; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java index 7dad5ea918d..94a74eefab5 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java @@ -19,12 +19,11 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright 2008 Peter Bray + * Portions Copyright (c) 2017-2018, Chris Fraire . */ -/* Portions Copyright 2008 Peter Bray */ package org.opengrok.indexer.history; -import java.io.BufferedInputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -33,6 +32,7 @@ import java.util.logging.Logger; import java.util.zip.GZIPInputStream; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; /** * Adds access to to a Razor Repository @@ -218,7 +218,9 @@ boolean fileHasHistory(File file) { } @Override - InputStream getHistoryGet(String parent, String basename, String rev) { + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { + // @TODO : Rename & Delete Support try { File binaryFile @@ -230,26 +232,37 @@ InputStream getHistoryGet(String parent, String basename, String rev) { // implementation will be useful to sites using GZIP as a // UNIX Compress replacement (A supported configuration // according to to the Razor 4.x/5.x manuals) - return new GZIPInputStream(new FileInputStream(binaryFile)); + try (FileInputStream in = new FileInputStream(binaryFile)) { + GZIPInputStream gzIn = new GZIPInputStream(in); + copyBytes(sink, gzIn); + } + return true; } File rcsFile = getRazorArchiveRCSFileFor(new File(parent, basename)); if (rcsFile != null && rcsFile.exists()) { String rcsPath = rcsFile.getPath(); - return new BufferedInputStream(new RCSget(rcsPath, rev)); + try (InputStream in = new RCSget(rcsPath, rev)) { + copyBytes(sink, in); + } + return true; } File sccsFile = getRazorArchiveSCCSFileFor(new File(parent, basename)); if (sccsFile != null && sccsFile.exists()) { ensureCommand(SCCSRepository.CMD_PROPERTY_KEY, SCCSRepository.CMD_FALLBACK); - return SCCSget.getRevision(RepoCommand, sccsFile, rev); + try (InputStream in = SCCSget.getRevision(RepoCommand, + sccsFile, rev)) { + copyBytes(sink, in); + } + return true; } } catch (Exception e) { LOGGER.log(Level.SEVERE, "getHistoryGet( " + parent + ", " + basename + ", " + rev + ")", e); } - return null; + return false; } @Override @@ -281,12 +294,7 @@ boolean fileHasAnnotation(File file) { } mappedFile = getRazorArchiveSCCSFileFor(file); - if (mappedFile.exists() && mappedFile.isFile()) { - return true; - } - - return false; - + return mappedFile.exists() && mappedFile.isFile(); } catch (Exception e) { return false; } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java index dcdce8af02d..6acbfba00d0 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java @@ -19,17 +19,17 @@ /* * Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved. - */ -/* * Copyright (c) 2010, Trond Norbye . All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.List; + +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -108,7 +108,8 @@ History getHistory(File file) { } @Override - InputStream getHistoryGet(String parent, String basename, String rev) { + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { throw new UnsupportedOperationException("Should never be called!"); } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java index 6b320c15017..43021546e29 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java @@ -23,11 +23,12 @@ */ package org.opengrok.indexer.history; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; import java.text.DateFormat; import java.text.FieldPosition; import java.text.ParseException; @@ -195,10 +196,8 @@ void removeAndVerifyOldestChangeset(List entries, } /** - * Gets the contents of a specific version of a named file into the - * specified target using - * {@link #getHistoryGet(java.lang.String, java.lang.String, java.lang.String)}. - * Subclasses can override to avoid the in-memory copy of the contents. + * Gets the contents of a specific version of a named file, and copies + * into the specified target. * * @param target a required target file which will be overwritten * @param parent the name of the directory containing the file @@ -207,34 +206,43 @@ void removeAndVerifyOldestChangeset(List entries, * @return {@code true} if contents were found * @throws java.io.IOException if an I/O error occurs */ - public boolean getHistoryGet(File target, String parent, String basename, - String rev) throws IOException { - try (InputStream in = getHistoryGet(parent, basename, rev)) { - if (in == null) { - return false; - } - try (OutputStream out = new FileOutputStream(target)) { - byte[] buf = new byte[8 * 1024]; - int n; - while ((n = in.read(buf)) != -1) { - out.write(buf, 0, n); - } - } - return true; + public boolean getHistoryGet( + File target, String parent, String basename, String rev) + throws IOException { + try (FileOutputStream out = new FileOutputStream(target)) { + return getHistoryGet(out::write, parent, basename, rev); + } + } + + /** + * Gets an {@link InputStream} of the contents of a specific version of a + * named file. + * @param parent the name of the directory containing the file + * @param basename the name of the file to get + * @param rev the revision to get + * @return a defined instance if contents were found; or else {@code null} + */ + public InputStream getHistoryGet( + String parent, String basename, String rev) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + if (getHistoryGet(out::write, parent, basename, rev)) { + return new ByteArrayInputStream(out.toByteArray()); } + return null; } /** - * Get an input stream that I may use to read a specific version of a named - * file. + * Subclasses must override to get the contents of a specific version of a + * named file, and copy to the specified {@code sink}. * + * @param sink a defined instance * @param parent the name of the directory containing the file * @param basename the name of the file to get * @param rev the revision to get - * @return An input stream containing the correct revision. + * @return a value indicating if the get was successful. */ - abstract InputStream getHistoryGet( - String parent, String basename, String rev); + abstract boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev); /** * Checks whether this parser can annotate files. @@ -584,23 +592,20 @@ protected String getRepoRelativePath(final File file) } /** - * Transfers the {@code executor} output to the {@code sink}. - * @return the number of calls to {@code sink} + * Copies all bytes from {@code in} to the {@code sink}. + * @return the number of writes to {@code sink} */ - static int transferExecutorOutput(BufferSink sink, Executor executor) - throws IOException { - try (InputStream in = executor.getOutputStream()) { - byte[] buffer = new byte[8 * 1024]; - int iterations = 0; - int len; - while ((len = in.read(buffer)) != -1) { - if (len > 0) { - ++iterations; - sink.write(buffer, 0, len); - } + static int copyBytes(BufferSink sink, InputStream in) throws IOException { + byte[] buffer = new byte[8 * 1024]; + int iterations = 0; + int len; + while ((len = in.read(buffer)) != -1) { + if (len > 0) { + ++iterations; + sink.write(buffer, 0, len); } - return iterations; } + return iterations; } class HistoryRevResult { diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java index b0520a76246..9efea759031 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.indexer.history; @@ -36,6 +37,7 @@ import java.util.logging.Logger; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -69,18 +71,22 @@ public SCCSRepository() { } @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { try { File history = SCCSHistoryParser.getSCCSFile(parent, basename); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); - return SCCSget.getRevision(RepoCommand, history, rev); + try (InputStream in = SCCSget.getRevision(RepoCommand, history, rev)) { + copyBytes(sink, in); + } + return true; } catch (FileNotFoundException ex) { - return null; + // continue below } catch (IOException ex) { LOGGER.log(Level.WARNING, "An error occurred while getting revision", ex); - return null; } + return false; } private Map getAuthors(File file) throws IOException { diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java index ce6e4e0f925..84270baa212 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java @@ -19,16 +19,15 @@ /* * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.indexer.history; -import java.io.BufferedInputStream; import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; import java.io.FileReader; import java.io.IOException; -import java.io.InputStream; import java.io.Reader; import java.nio.file.Files; import java.util.ArrayList; @@ -41,6 +40,7 @@ import java.util.regex.Pattern; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; /** @@ -169,8 +169,8 @@ History getHistory(File file, String sinceRevision) } @Override - InputStream getHistoryGet(String parent, final String basename, String rev) { - InputStream ret = null; + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { File directory = new File(parent); @@ -206,35 +206,33 @@ InputStream getHistoryGet(String parent, final String basename, String rev) { LOGGER.log(Level.WARNING, "Failed get revision {2} for: \"{0}\" Exit code: {1}", new Object[]{new File(parent, basename).getAbsolutePath(), String.valueOf(status), rev}); - return null; + return false; } - ret = new BufferedInputStream(new FileInputStream(new File(tmp, basename))) { - - @Override - public void close() throws IOException { - super.close(); - boolean deleteOnExit = false; - File tmpFile = new File(tmp, basename); - // delete the temporary file on close - if (!tmpFile.delete()) { - // try on JVM exit - deleteOnExit = true; - tmpFile.deleteOnExit(); - } - // delete the temporary directory on close - if (deleteOnExit || !tmp.delete()) { - // try on JVM exit - tmp.deleteOnExit(); - } + File tmpFile = new File(tmp, basename); + try (FileInputStream in = new FileInputStream(tmpFile)) { + copyBytes(sink, in); + } finally { + boolean deleteOnExit = false; + // delete the temporary file on close + if (!tmpFile.delete()) { + // try on JVM exit + deleteOnExit = true; + tmpFile.deleteOnExit(); + } + // delete the temporary directory on close + if (deleteOnExit || !tmp.delete()) { + // try on JVM exit + tmp.deleteOnExit(); } - }; + } + return true; } catch (IOException exp) { LOGGER.log(Level.SEVERE, "Failed to get file: " + exp.getClass().toString(), exp); } - return ret; + return false; } @Override diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java index fcf473cfb92..770b0ce1e62 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java @@ -19,13 +19,12 @@ /* * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017-2018, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.List; import java.util.logging.Level; @@ -35,6 +34,7 @@ import javax.xml.parsers.ParserConfigurationException; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.BufferSink; import org.opengrok.indexer.util.Executor; import org.w3c.dom.Document; import org.w3c.dom.Node; @@ -231,8 +231,8 @@ Executor getHistoryLogExecutor(final File file, String sinceRevision, } @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { - InputStream ret = null; + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { File directory = new File(getDirectoryName()); @@ -242,7 +242,7 @@ public InputStream getHistoryGet(String parent, String basename, String rev) { } catch (IOException exp) { LOGGER.log(Level.SEVERE, "Failed to get canonical path: {0}", exp.getClass().toString()); - return null; + return false; } String filename = filepath.substring(getDirectoryName().length() + 1); @@ -259,10 +259,16 @@ public InputStream getHistoryGet(String parent, String basename, String rev) { Executor executor = new Executor(cmd, directory, RuntimeEnvironment.getInstance().getInteractiveCommandTimeout()); if (executor.exec() == 0) { - ret = executor.getOutputStream(); + try { + copyBytes(sink, executor.getOutputStream()); + return true; + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Failed to get content for {0}", + basename); + } } - return ret; + return false; } @Override diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java index ef9e10401a9..fdb7720ba78 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java @@ -19,17 +19,17 @@ /* * Copyright (c) 2017, 2018 Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2018, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.File; import java.io.IOException; -import java.io.InputStream; -import java.text.DateFormat; import java.text.ParseException; import java.util.Arrays; import org.junit.Assert; import org.junit.Test; +import org.opengrok.indexer.util.BufferSink; /** * @@ -158,8 +158,9 @@ public History getHistory(File file) throws HistoryException { } @Override - public InputStream getHistoryGet(String parent, String basename, String rev) { - return null; + boolean getHistoryGet( + BufferSink sink, String parent, String basename, String rev) { + return false; } @Override From e861a905b107c5b093cd136cbb25d2cd20af81a6 Mon Sep 17 00:00:00 2001 From: Chris Fraire Date: Thu, 20 Dec 2018 09:58:04 -0600 Subject: [PATCH 4/4] Make static class --- .../java/org/opengrok/indexer/history/BazaarRepository.java | 5 +---- .../main/java/org/opengrok/indexer/history/Repository.java | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java index 66469579c9a..e9edeefe0a7 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java @@ -25,7 +25,6 @@ import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.List; import java.util.TreeSet; @@ -106,9 +105,7 @@ boolean getHistoryGet( ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); String[] argv = {RepoCommand, "cat", "-r", rev, filename}; process = Runtime.getRuntime().exec(argv, null, directory); - try (InputStream in = process.getInputStream()) { - copyBytes(sink, in); - } + copyBytes(sink, process.getInputStream()); return true; } catch (Exception exp) { LOGGER.log(Level.SEVERE, diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java index 43021546e29..5b7bec4841b 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java @@ -608,7 +608,7 @@ static int copyBytes(BufferSink sink, InputStream in) throws IOException { return iterations; } - class HistoryRevResult { + static class HistoryRevResult { public boolean success; public int iterations; }