Reduce synchronization in Lucene indexer

Change-Id: I5c1ff60975fd68b03d5ed97c05909f807526c3ec
Signed-off-by: Dawid Pakuła <zulus@w3des.net>
diff --git a/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/IndexContainer.java b/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/IndexContainer.java
index c13f08c..bad4af5 100644
--- a/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/IndexContainer.java
+++ b/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/IndexContainer.java
@@ -12,7 +12,6 @@
 package org.eclipse.dltk.internal.core.index.lucene;
 
 import java.io.IOException;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.HashMap;
@@ -111,31 +110,12 @@
 				new HashMap<Integer, SearcherManager>());
 	}
 
-	private void purgeLocks(Path path) {
-		/*
-		 * Checks if any write locks exist (might be not removed if JVM crashed
-		 * or was terminated abnormally) and simply deletes them.
-		 */
-		Path writeLockPath = path.resolve(IndexWriter.WRITE_LOCK_NAME);
-		if (writeLockPath.toFile().exists()) {
-			try {
-				Files.delete(writeLockPath);
-			} catch (IOException e) {
-				Logger.logException(e);
-			}
-		}
-	}
-
 	private IndexWriter createWriter(Path path) throws IOException {
 
 		Directory indexDir = FSDirectory.open(path,
 				new SingleInstanceLockFactory());
-		purgeLocks(path);
 		IndexWriterConfig config = new IndexWriterConfig(new SimpleAnalyzer());
 		config.setUseCompoundFile(true);
-		// we already are on separate thread
-		// SerialMergeScheduler sheduler = new SerialMergeScheduler();
-		// config.setMergeScheduler(sheduler);
 		config.setOpenMode(OpenMode.CREATE_OR_APPEND);
 		config.setCommitOnClose(false);
 		return new IndexWriter(indexDir, config);
@@ -207,7 +187,6 @@
 					Path writerPath = getPath(dataType, elementType);
 					writer = getWriter(writerPath);
 					fIndexWriters.get(dataType).put(elementType, writer);
-					fIndexSearchers.get(dataType).put(elementType, null);
 				}
 			}
 		}
@@ -220,7 +199,6 @@
 		SearcherManager searcher = fIndexSearchers.get(dataType)
 				.get(elementType);
 		try {
-			boolean refresh = true;
 			if (searcher == null) {
 				synchronized (fIndexSearchers) {
 					searcher = fIndexSearchers.get(dataType).get(elementType);
@@ -230,14 +208,10 @@
 								new SearcherFactory());
 						fIndexSearchers.get(dataType).put(elementType,
 								searcher);
-						refresh = false;
 					}
 
 				}
 			}
-			if (refresh) {
-				searcher.maybeRefreshBlocking();
-			}
 		} catch (IndexNotFoundException e) {
 			return null;
 		} catch (IOException e) {
@@ -330,4 +304,32 @@
 			Logger.logException(e);
 		}
 	}
+
+	public IndexContainer refresh(boolean block) {
+		try {
+			Thread.sleep(100);
+		} catch (InterruptedException e1) {
+		}
+		synchronized (fIndexSearchers) {
+			for (Map<Integer, SearcherManager> searcher : fIndexSearchers
+					.values()) {
+				for (SearcherManager man : searcher.values()) {
+					try {
+						if (man != null) {
+							if (block) {
+								man.maybeRefreshBlocking();
+							} else {
+								man.maybeRefresh();
+							}
+						}
+					} catch (IOException e) {
+						Logger.logException(e);
+					}
+				}
+			}
+		}
+		return this;
+
+	}
+
 }
diff --git a/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneIndexer.java b/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneIndexer.java
index c7aa42c..43e9d04 100644
--- a/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneIndexer.java
+++ b/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneIndexer.java
@@ -179,6 +179,7 @@
 	public void removeDocument(IPath containerPath, String sourceModulePath) {
 		LuceneManager.INSTANCE.delete(containerPath.toString(),
 				sourceModulePath);
+
 	}
 
 	private void resetDocument(ISourceModule sourceModule,
diff --git a/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneManager.java b/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneManager.java
index 8f72b45..d5e4aca 100644
--- a/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneManager.java
+++ b/core/plugins/org.eclipse.dltk.core.index.lucene/src/org/eclipse/dltk/internal/core/index/lucene/LuceneManager.java
@@ -18,25 +18,19 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
 import java.util.UUID;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ForkJoinTask;
 
 import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.search.SearcherManager;
 import org.eclipse.core.runtime.IPath;
-import org.eclipse.core.runtime.IProgressMonitor;
-import org.eclipse.core.runtime.IStatus;
-import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.Platform;
-import org.eclipse.core.runtime.Status;
-import org.eclipse.core.runtime.SubMonitor;
-import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.dltk.core.DLTKLanguageManager;
 import org.eclipse.dltk.core.IDLTKLanguageToolkit;
 import org.eclipse.dltk.core.IShutdownListener;
@@ -74,82 +68,10 @@
 	 */
 	INSTANCE;
 
-	private final class Committer extends Job {
-
-		private final static int DELAY = 50;
-		private boolean fClosed = false;
-
-		public Committer() {
-			super(""); //$NON-NLS-1$
-			setUser(false);
-			setSystem(true);
-		}
-
-		@Override
-		public IStatus run(IProgressMonitor monitor) {
-			// Get containers with uncommitted changes only
-			List<IndexContainer> dirtyContainers = getDirtyContainers();
-			if (dirtyContainers.isEmpty()) {
-				return Status.CANCEL_STATUS;
-			}
-			int containersNumber = dirtyContainers.size();
-			SubMonitor subMonitor = SubMonitor.convert(monitor,
-					containersNumber);
-			try {
-				if (subMonitor.isCanceled()) {
-					return Status.CANCEL_STATUS;
-				}
-				for (IndexContainer indexContainer : dirtyContainers) {
-					ForkJoinTask.adapt(() -> {
-						indexContainer.commit();
-					}).fork();
-				}
-			} catch (Exception e) {
-				Logger.logException(e);
-			} finally {
-				subMonitor.done();
-			}
-			return Status.OK_STATUS;
-		}
-
-		@Override
-		public boolean belongsTo(Object family) {
-			return family == LucenePlugin.LUCENE_JOB_FAMILY;
-		}
-
-		synchronized void commit() {
-			if (fClosed) {
-				return;
-			}
-			int currentState = getState();
-			if (currentState == NONE) {
-				schedule(DELAY);
-			} else if (currentState == SLEEPING) {
-				wakeUp(DELAY);
-			} else if (currentState == WAITING) {
-				sleep();
-				wakeUp(DELAY);
-			} else {
-				cancel();
-				schedule(DELAY);
-			}
-		}
-
-		synchronized void close() {
-			if (!fClosed) {
-				cancel();
-				fClosed = true;
-			}
-		}
-
-	}
-
 	private final class ShutdownListener implements IShutdownListener {
 
 		@Override
 		public void shutdown() {
-			// Close background committer if it is not already closed
-			fCommitter.close();
 			// Shutdown manager
 			LuceneManager.INSTANCE.shutdown();
 		}
@@ -160,11 +82,11 @@
 
 		@Override
 		public void aboutToBeIdle() {
+			commit();
 		}
 
 		@Override
 		public void aboutToBeRun(long idlingTime) {
-			fCommitter.run(new NullProgressMonitor());
 		}
 
 	}
@@ -175,15 +97,25 @@
 
 	private final String fIndexRoot;
 	private final Properties fIndexProperties;
-	private final Properties fContainerMappings;
+	private final Map<String, String> fContainerMappings;
 	private final Map<String, IndexContainer> fIndexContainers;
-	private final Committer fCommitter;
+
+	private void commit() {
+		try {
+			for (IndexContainer indexContainer : getDirtyContainers()) {
+				indexContainer.commit();
+				indexContainer.refresh(true);
+			}
+
+		} catch (Exception e) {
+			Logger.logException(e);
+		}
+	}
 
 	private LuceneManager() {
 		fIndexProperties = new Properties();
-		fContainerMappings = new Properties();
-		fIndexContainers = new ConcurrentHashMap<>();
-		fCommitter = new Committer();
+		fContainerMappings = new HashMap<>();
+		fIndexContainers = new HashMap<>();
 		fIndexRoot = Platform
 				.getStateLocation(LucenePlugin.getDefault().getBundle())
 				.append(INDEX_DIR).toOSString();
@@ -261,7 +193,7 @@
 	 * @param sourceModule
 	 */
 	public final void delete(String container, String sourceModule) {
-		if (fContainerMappings.getProperty(container) != null) {
+		if (fContainerMappings.get(container) != null) {
 			getIndexContainer(container).delete(sourceModule);
 		}
 	}
@@ -280,18 +212,21 @@
 	}
 
 	private IndexContainer getIndexContainer(String container) {
-		String containerId = fContainerMappings.getProperty(container);
+		String containerId = fContainerMappings.get(container);
 		if (containerId == null) {
 			synchronized (fContainerMappings) {
-				do {
-					// Just to be sure that ID does not already exist
-					containerId = UUID.randomUUID().toString();
-				} while (fContainerMappings.containsValue(containerId));
-				fContainerMappings.put(container, containerId);
-				fIndexContainers.put(containerId,
-						new IndexContainer(fIndexRoot, containerId));
-				// Persist mapping
-				saveMappings();
+				containerId = fContainerMappings.get(container);
+				if (containerId == null) {
+					do {
+						// Just to be sure that ID does not already exist
+						containerId = UUID.randomUUID().toString();
+					} while (fContainerMappings.containsValue(containerId));
+					fContainerMappings.put(container, containerId);
+					fIndexContainers.put(containerId,
+							new IndexContainer(fIndexRoot, containerId));
+					// Persist mapping
+					saveMappings();
+				}
 			}
 		}
 		return fIndexContainers.get(containerId);
@@ -299,7 +234,7 @@
 
 	private void deleteIndexContainer(String container, boolean wait) {
 		synchronized (fContainerMappings) {
-			String containerId = (String) fContainerMappings.remove(container);
+			String containerId = fContainerMappings.remove(container);
 			if (containerId != null) {
 				IndexContainer containerEntry = fIndexContainers
 						.remove(containerId);
@@ -346,10 +281,12 @@
 	}
 
 	private void registerIndexContainers() {
-		for (String container : fContainerMappings.stringPropertyNames()) {
-			String containerId = fContainerMappings.getProperty(container);
-			fIndexContainers.put(containerId,
-					new IndexContainer(fIndexRoot, containerId));
+		synchronized (fContainerMappings) {
+
+			for (String containerId : fContainerMappings.values()) {
+				fIndexContainers.put(containerId,
+						new IndexContainer(fIndexRoot, containerId));
+			}
 		}
 	}
 
@@ -370,11 +307,19 @@
 		if (!file.exists()) {
 			return;
 		}
-		try (FileInputStream fis = new FileInputStream(file)) {
-			fContainerMappings.load(fis);
-		} catch (IOException e) {
-			Logger.logException(e);
+		synchronized (fContainerMappings) {
+			try (FileInputStream fis = new FileInputStream(file)) {
+				Properties p = new Properties();
+				p.load(fis);
+				for (Entry<Object, Object> entry : p.entrySet()) {
+					fContainerMappings.put((String) entry.getKey(),
+							(String) entry.getValue());
+				}
+			} catch (IOException e) {
+				Logger.logException(e);
+			}
 		}
+
 	}
 
 	private void saveProperties() {
@@ -388,11 +333,20 @@
 
 	private void saveMappings() {
 		File file = Paths.get(fIndexRoot, MAPPINGS_FILE).toFile();
-		try (FileOutputStream fos = new FileOutputStream(file)) {
-			fContainerMappings.store(fos, ""); //$NON-NLS-1$
-		} catch (IOException e) {
-			Logger.logException(e);
+		synchronized (fContainerMappings) {
+			try (FileOutputStream fos = new FileOutputStream(file)) {
+				Properties p = new Properties();
+				for (Entry<String, String> entry : fContainerMappings
+						.entrySet()) {
+					p.setProperty(entry.getKey(), entry.getValue());
+				}
+
+				p.store(fos, ""); //$NON-NLS-1$
+			} catch (IOException e) {
+				Logger.logException(e);
+			}
 		}
+
 	}
 
 	private void resetProperties() {
@@ -418,12 +372,14 @@
 		 * up the related mappings that might left.
 		 */
 		Set<String> toRemove = new HashSet<>();
-		for (String mappedContainer : fContainerMappings
-				.stringPropertyNames()) {
-			if (!containers.contains(mappedContainer)) {
-				toRemove.add(mappedContainer);
+		synchronized (fContainerMappings) {
+			for (String mappedContainer : fContainerMappings.values()) {
+				if (!containers.contains(mappedContainer)) {
+					toRemove.add(mappedContainer);
+				}
 			}
 		}
+
 		if (!toRemove.isEmpty()) {
 			for (String container : toRemove) {
 				deleteIndexContainer(container, true);
@@ -461,4 +417,8 @@
 		}
 		indexRoot.toFile().mkdir();
 	}
+
+	public void refreshContainer(String fContainer, boolean block) {
+		getIndexContainer(fContainer).refresh(block);
+	}
 }