Skip to content

Commit

Permalink
Improve performance by throttling database change events (#5843)
Browse files Browse the repository at this point in the history
* Improve performance by throttling database change events

* Move throttle to logic
  • Loading branch information
tobiasdiez authored Jan 18, 2020
1 parent d78e48f commit 6319d05
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We fixed an issue where the Medline fetcher was only working when JabRef was running from source. [#5645](https://github.com/JabRef/jabref/issues/5645)
- We fixed some visual issues in the dark theme. [#5764](https://github.com/JabRef/jabref/pull/5764) [#5753](https://github.com/JabRef/jabref/issues/5753)
- We fixed an issue where non-default previews didn't handle unicode characters. [#5779](https://github.com/JabRef/jabref/issues/5779)
- We improved the performance, especially changing field values in the entry should feel smoother now.
- We fixed an issue where the ampersand character wasn't rendering correctly on previews. [#3840](https://github.com/JabRef/jabref/issues/3840)
- We fixed an issue where an erroneous "The library has been modified by another program" message was shown when saving. [#4877](https://github.com/JabRef/jabref/issues/4877)
- We fixed an issue where the file extension was missing after downloading a file (we now fall-back to pdf). [#5816](https://github.com/JabRef/jabref/issues/5816)
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.groups.DefaultGroupsFactory;
import org.jabref.logic.layout.format.LatexToUnicodeFormatter;
import org.jabref.logic.util.DelayTaskThrottler;
import org.jabref.model.FieldChange;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -56,6 +57,7 @@ public class GroupNodeViewModel {
private final TaskExecutor taskExecutor;
private final CustomLocalDragboard localDragBoard;
private final ObservableList<BibEntry> entriesList;
private final DelayTaskThrottler throttler;

public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager stateManager, TaskExecutor taskExecutor, GroupTreeNode groupNode, CustomLocalDragboard localDragBoard) {
this.databaseContext = Objects.requireNonNull(databaseContext);
Expand Down Expand Up @@ -88,6 +90,7 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state
// The wrapper created by the FXCollections will set a weak listener on the wrapped list. This weak listener gets garbage collected. Hence, we need to maintain a reference to this list.
entriesList = databaseContext.getDatabase().getEntries();
entriesList.addListener(this::onDatabaseChanged);
throttler = new DelayTaskThrottler(1000);

ObservableList<Boolean> selectedEntriesMatchStatus = EasyBind.map(stateManager.getSelectedEntries(), groupNode::matches);
anySelectedEntriesMatched = BindingsHelper.any(selectedEntriesMatchStatus, matched -> matched);
Expand Down Expand Up @@ -218,7 +221,7 @@ public GroupTreeNode getGroupNode() {
* Gets invoked if an entry in the current database changes.
*/
private void onDatabaseChanged(ListChangeListener.Change<? extends BibEntry> change) {
calculateNumberOfMatches();
throttler.schedule(this::calculateNumberOfMatches);
}

private void calculateNumberOfMatches() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@

import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.Future;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import org.jabref.logic.util.DelayTaskThrottler;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.AutosaveEvent;
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
Expand All @@ -25,40 +23,34 @@
public class AutosaveManager {

private static final Logger LOGGER = LoggerFactory.getLogger(AutosaveManager.class);
private static final int AUTO_SAVE_DELAY = 200;

private static Set<AutosaveManager> runningInstances = new HashSet<>();

private final BibDatabaseContext bibDatabaseContext;
private final ScheduledExecutorService executor;

private final EventBus eventBus;
private final CoarseChangeFilter changeFilter;
private Future<?> scheduledSaveAction;
private final DelayTaskThrottler throttler;

private AutosaveManager(BibDatabaseContext bibDatabaseContext) {
this.bibDatabaseContext = bibDatabaseContext;
ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1);
executor.setRemoveOnCancelPolicy(true); // This prevents memory leaks
this.executor = executor;
this.throttler = new DelayTaskThrottler(2000);
this.eventBus = new EventBus();
this.changeFilter = new CoarseChangeFilter(bibDatabaseContext);
changeFilter.registerListener(this);
}

@Subscribe
public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) {
if (scheduledSaveAction != null) {
scheduledSaveAction.cancel(false);
}
scheduledSaveAction = executor.schedule(() -> {
throttler.schedule(() -> {
eventBus.post(new AutosaveEvent());
}, AUTO_SAVE_DELAY, TimeUnit.MILLISECONDS);
});
}

private void shutdown() {
changeFilter.unregisterListener(this);
changeFilter.shutdown();
executor.shutdown();
throttler.shutdown();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,14 @@
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import org.jabref.logic.bibtex.InvalidFieldValueException;
import org.jabref.logic.exporter.AtomicFileWriter;
import org.jabref.logic.exporter.BibtexDatabaseWriter;
import org.jabref.logic.exporter.SavePreferences;
import org.jabref.logic.util.DelayTaskThrottler;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
Expand Down Expand Up @@ -46,17 +43,15 @@ public class BackupManager {

private final BibDatabaseContext bibDatabaseContext;
private final JabRefPreferences preferences;
private final ExecutorService executor;
private final Runnable backupTask = () -> determineBackupPath().ifPresent(this::performBackup);
private final DelayTaskThrottler throttler;
private final CoarseChangeFilter changeFilter;
private final BibEntryTypesManager entryTypesManager;

private BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, JabRefPreferences preferences) {
this.bibDatabaseContext = bibDatabaseContext;
this.entryTypesManager = entryTypesManager;
this.preferences = preferences;
BlockingQueue<Runnable> workerQueue = new ArrayBlockingQueue<>(1);
this.executor = new ThreadPoolExecutor(1, 1, 0, TimeUnit.SECONDS, workerQueue);
this.throttler = new DelayTaskThrottler(15000);

changeFilter = new CoarseChangeFilter(bibDatabaseContext);
changeFilter.registerListener(this);
Expand All @@ -71,8 +66,6 @@ static Path getBackupPath(Path originalPath) {
* As long as no database file is present in {@link BibDatabaseContext}, the {@link BackupManager} will do nothing.
*
* @param bibDatabaseContext Associated {@link BibDatabaseContext}
* @param entryTypesManager
* @param preferences
*/
public static BackupManager start(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, JabRefPreferences preferences) {
BackupManager backupManager = new BackupManager(bibDatabaseContext, entryTypesManager, preferences);
Expand Down Expand Up @@ -151,11 +144,7 @@ public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextCh
}

private void startBackupTask() {
try {
executor.submit(backupTask);
} catch (RejectedExecutionException e) {
LOGGER.debug("Rejecting while another backup process is already running.");
}
throttler.schedule(() -> determineBackupPath().ifPresent(this::performBackup));
}

/**
Expand All @@ -165,7 +154,7 @@ private void startBackupTask() {
private void shutdown() {
changeFilter.unregisterListener(this);
changeFilter.shutdown();
executor.shutdown();
throttler.shutdown();
determineBackupPath().ifPresent(this::deleteBackupFile);
}

Expand Down
51 changes: 51 additions & 0 deletions src/main/java/org/jabref/logic/util/DelayTaskThrottler.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package org.jabref.logic.util;

import java.util.concurrent.Future;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This class allows to throttle a list of tasks.
* Use case: you have an event that occurs often, and every time you want to invoke the same task.
* However, if a lot of events happen in a relatively short time span, then only one task should be invoked.
*
* @implNote Once {@link #schedule(Runnable)} is called, the task is delayed for a given time span.
* If during this time, {@link #schedule(Runnable)} is called again, then the original task is canceled and the new one scheduled.
*/
public class DelayTaskThrottler {

private static final Logger LOGGER = LoggerFactory.getLogger(DelayTaskThrottler.class);

private final ScheduledThreadPoolExecutor executor;
private final int delay;

private Future<?> scheduledTask;

/**
* @param delay delay in milliseconds
*/
public DelayTaskThrottler(int delay) {
this.delay = delay;
this.executor = new ScheduledThreadPoolExecutor(1);
this.executor.setRemoveOnCancelPolicy(true);
}

public void schedule(Runnable command) {
if (scheduledTask != null) {
scheduledTask.cancel(false);
}
try {
scheduledTask = executor.schedule(command, delay, TimeUnit.MILLISECONDS);
} catch (RejectedExecutionException e) {
LOGGER.debug("Rejecting while another process is already running.");
}
}

public void shutdown() {
executor.shutdown();
}
}

0 comments on commit 6319d05

Please sign in to comment.