Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fix for issue 2831 #3721

Merged
merged 20 commits into from
Feb 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
## [Unreleased]

### Changed
- Abbreviate journal names functionality is now running parallel, increasing performance significantly. [#2831] (https://github.com/JabRef/jabref/issues/2831)
- Changed ID-based entry generator to store the last used fetcher. [#2796] (https://github.com/JabRef/jabref/issues/2796)
- Reorganised annotation information on the right side of the "File annotations" tab. [#3109](https://github.com/JabRef/jabref/issues/3109)
- We now show a small notification icon in the entry editor when we detect data inconsistency or other problems. [#3145](https://github.com/JabRef/jabref/issues/3145)
Expand Down
15 changes: 15 additions & 0 deletions scripts/bib-file-generator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
number_of_entries = 100000
file = open("generatedDatabase.bib", 'w')

for i in range(number_of_entries):
entry = """@article{%i,
author = {%i},
title = {%i},
journal = {%i},
volume = {%i},
year = {%i},
pages = {%i},
}""" % (i, i, i, i, i, i, i)
file.write(entry)
file.flush()
file.close()
63 changes: 41 additions & 22 deletions src/main/java/org/jabref/JabRefExecutorService.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
package org.jabref;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.Callable;
Expand All @@ -12,7 +17,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;


/**
* Responsible for managing of all threads (except Swing threads) in JabRef
*/
Expand All @@ -36,24 +40,17 @@ public class JabRefExecutorService implements Executor {
private final Timer timer = new Timer("timer", true);
private Thread remoteThread;

private JabRefExecutorService() { }
private JabRefExecutorService() {
}

@Override
public void execute(Runnable command) {
if (command == null) {
LOGGER.debug("Received null as command for execution");
return;
}

Objects.requireNonNull(command);
executorService.execute(command);
}

public void executeAndWait(Runnable command) {
if (command == null) {
LOGGER.debug("Received null as command for execution");
return;
}

Objects.requireNonNull(command);
Future<?> future = executorService.submit(command);
while (true) {
try {
Expand All @@ -67,12 +64,8 @@ public void executeAndWait(Runnable command) {
}
}

public boolean executeAndWait(Callable command) {
if (command == null) {
LOGGER.debug("Received null as command for execution");
return false;
}

public boolean executeAndWait(Callable<?> command) {
Objects.requireNonNull(command);
Future<?> future = executorService.submit(command);
while (true) {
try {
Expand All @@ -87,6 +80,35 @@ public boolean executeAndWait(Callable command) {
}
}

/**
* Executes a callable task that provides a return value after the calculation is done.
*
* @param command The task to execute.
* @return A Future object that provides the returning value.
*/
public <T> Future<T> execute(Callable<T> command) {
Objects.requireNonNull(command);
return executorService.submit(command);
}

/**
* Executes a collection of callable tasks and returns a List of the resulting Future objects after the calculation is done.
*
* @param tasks The tasks to execute
* @return A List of Future objects that provide the returning values.
*/
public <T> List<Future<T>> executeAll(Collection<Callable<T>> tasks) {
Objects.requireNonNull(tasks);
List<Future<T>> futures = new ArrayList<>();
try {
futures = executorService.invokeAll(tasks);
} catch (InterruptedException exception) {
LOGGER.error("Unable to execute tasks", exception);
return Collections.emptyList();
}
return futures;
}

public void executeInterruptableTask(final Runnable runnable) {
this.lowPriorityExecutorService.execute(runnable);
}
Expand All @@ -96,10 +118,7 @@ public void executeInterruptableTask(final Runnable runnable, String taskName) {
}

public void executeInterruptableTaskAndWait(Runnable runnable) {
if (runnable == null) {
LOGGER.debug("Received null as command for execution");
return;
}
Objects.requireNonNull(runnable);

Future<?> future = lowPriorityExecutorService.submit(runnable);
while (true) {
Expand Down
30 changes: 23 additions & 7 deletions src/main/java/org/jabref/gui/journals/AbbreviateAction.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package org.jabref.gui.journals;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;

import org.jabref.Globals;
import org.jabref.JabRefExecutorService;
Expand All @@ -12,12 +16,16 @@
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.InternalBibtexFields;

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

/**
* Converts journal full names to either iso or medline abbreviations for all
* selected entries.
*/
public class AbbreviateAction extends AbstractWorker {

private static final Logger LOGGER = LoggerFactory.getLogger(AbbreviateAction.class);
private final BasePanel panel;
private String message = "";
private final boolean iso;
Expand All @@ -35,30 +43,38 @@ public void init() {
@Override
public void run() {
List<BibEntry> entries = panel.getSelectedEntries();

UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(
Globals.journalAbbreviationLoader.getRepository(Globals.prefs.getJournalAbbreviationPreferences()),
iso);

NamedCompound ce = new NamedCompound(Localization.lang("Abbreviate journal names"));
int count = 0;
Set<Callable<Boolean>> tasks = new HashSet<>();

// Collect all callables to execute in one collection.
for (BibEntry entry : entries) {
Callable<Boolean> callable = () -> {
for (String journalField : InternalBibtexFields.getJournalNameFields()) {
if (undoableAbbreviator.abbreviate(panel.getDatabase(), entry, journalField, ce)) {
return true;
}
}

return false;
};
tasks.add(callable);
}

boolean result = JabRefExecutorService.INSTANCE.executeAndWait(callable);
if (result) {
count++;
// Execute the callables and wait for the results.
List<Future<Boolean>> futures = JabRefExecutorService.INSTANCE.executeAll(tasks);

// Evaluate the results of the callables.
long count = futures.stream().filter(future -> {
try {
return future.get();
} catch (InterruptedException | ExecutionException exception) {
LOGGER.error("Unable to retrieve value.", exception);
return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not break the loop here, just continue with the next future. If the return false; makes sense here, please add a comment why false is returned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the return is for the lambda so everything is fine.

}
}
}).count();

if (count > 0) {
ce.end();
Expand Down