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

Improve performance of journal abbreviation loader #3009

Merged
merged 2 commits into from
Jul 13, 2017
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
28 changes: 13 additions & 15 deletions src/main/java/org/jabref/collab/FileUpdateMonitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,19 @@ public class FileUpdateMonitor implements Runnable {
private static final Log LOGGER = LogFactory.getLog(FileUpdateMonitor.class);

private static final int WAIT = 4000;

private int numberOfUpdateListener;
private final Map<String, Entry> entries = new HashMap<>();
private int numberOfUpdateListener;

private static synchronized Path getTempFile() {
Path temporaryFile = null;
try {
temporaryFile = Files.createTempFile("jabref", null);
temporaryFile.toFile().deleteOnExit();
} catch (IOException ex) {
LOGGER.warn("Could not create temporary file.", ex);
}
return temporaryFile;
}

@Override
public void run() {
Expand Down Expand Up @@ -119,7 +129,7 @@ public void updateTimeStamp(String key) {
* is used for comparison with the changed on-disk version.
* @param key String The handle for this monitor.
* @throws IllegalArgumentException If the handle doesn't correspond to an entry.
* @return File The temporary file.
* @return Path The temporary file.
*/
public Path getTempFile(String key) throws IllegalArgumentException {
Entry entry = entries.get(key);
Expand All @@ -129,7 +139,6 @@ public Path getTempFile(String key) throws IllegalArgumentException {
return entry.getTmpFile();
}


/**
* A class containing the File, the FileUpdateListener and the current time stamp for one file.
*/
Expand Down Expand Up @@ -209,15 +218,4 @@ public void decreaseTimeStamp() {
timeStamp--;
}
}

private static synchronized Path getTempFile() {
Path temporaryFile = null;
try {
temporaryFile = Files.createTempFile("jabref", null);
temporaryFile.toFile().deleteOnExit();
} catch (IOException ex) {
LOGGER.warn("Could not create temporary file.", ex);
}
return temporaryFile;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
import java.util.Set;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand All @@ -25,7 +27,7 @@ public class AbbreviationParser {

private static final Log LOGGER = LogFactory.getLog(AbbreviationParser.class);

private final List<Abbreviation> abbreviations = new LinkedList<>();
private final Set<Abbreviation> abbreviations = new HashSet<>(5000);

public void readJournalListFromResource(String resourceFileName) {
URL url = Objects.requireNonNull(JournalAbbreviationRepository.class.getResource(Objects.requireNonNull(resourceFileName)));
Expand Down Expand Up @@ -90,9 +92,7 @@ private void addLine(String line) {
}

Abbreviation abbreviation = new Abbreviation(fullName, abbrName);
if (!abbreviations.contains(abbreviation)) {
this.abbreviations.add(abbreviation);
}
this.abbreviations.add(abbreviation);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package org.jabref.logic.journals;

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.HashSet;
import java.util.Objects;
import java.util.Optional;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.Set;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand All @@ -19,32 +16,35 @@
public class JournalAbbreviationRepository {

private static final Log LOGGER = LogFactory.getLog(JournalAbbreviationRepository.class);
private final Map<String, Abbreviation> fullNameLowerCase2Abbreviation = new HashMap<>();
private final Map<String, Abbreviation> isoLowerCase2Abbreviation = new HashMap<>();

private final Map<String, Abbreviation> medlineLowerCase2Abbreviation = new HashMap<>();

private final SortedSet<Abbreviation> abbreviations = new TreeSet<>();
private final Set<Abbreviation> abbreviations = new HashSet<>(16000); // We have over 15.000 abbreviations in the built-in lists

public JournalAbbreviationRepository(Abbreviation... abbreviations) {
for (Abbreviation abbreviation : abbreviations) {
addEntry(abbreviation);
}
}

private static boolean isMatched(String name, Abbreviation abbreviation) {
return name.equalsIgnoreCase(abbreviation.getName())
|| name.equalsIgnoreCase(abbreviation.getIsoAbbreviation())
|| name.equalsIgnoreCase(abbreviation.getMedlineAbbreviation());
}

private static boolean isMatchedAbbreviated(String name, Abbreviation abbreviation) {
return name.equalsIgnoreCase(abbreviation.getIsoAbbreviation())
|| name.equalsIgnoreCase(abbreviation.getMedlineAbbreviation());
}

public int size() {
return abbreviations.size();
}

public boolean isKnownName(String journalName) {
String nameKey = Objects.requireNonNull(journalName).trim().toLowerCase(Locale.ENGLISH);
return (fullNameLowerCase2Abbreviation.containsKey(nameKey)) || (isoLowerCase2Abbreviation.containsKey(nameKey))
|| (medlineLowerCase2Abbreviation.containsKey(nameKey));
return abbreviations.stream().anyMatch(abbreviation -> isMatched(journalName.trim(), abbreviation));
}

public boolean isAbbreviatedName(String journalName) {
String nameKey = Objects.requireNonNull(journalName).trim().toLowerCase(Locale.ENGLISH);
return (isoLowerCase2Abbreviation.containsKey(nameKey)) || (medlineLowerCase2Abbreviation.containsKey(nameKey));
return abbreviations.stream().anyMatch(abbreviation -> isMatchedAbbreviated(journalName.trim(), abbreviation));
}

/**
Expand All @@ -54,75 +54,39 @@ public boolean isAbbreviatedName(String journalName) {
* @return The abbreviated name
*/
public Optional<Abbreviation> getAbbreviation(String journalName) {
String nameKey = Objects.requireNonNull(journalName).toLowerCase(Locale.ENGLISH).trim();

if (fullNameLowerCase2Abbreviation.containsKey(nameKey)) {
return Optional.of(fullNameLowerCase2Abbreviation.get(nameKey));
} else if (isoLowerCase2Abbreviation.containsKey(nameKey)) {
return Optional.of(isoLowerCase2Abbreviation.get(nameKey));
} else if (medlineLowerCase2Abbreviation.containsKey(nameKey)) {
return Optional.of(medlineLowerCase2Abbreviation.get(nameKey));
} else {
return Optional.empty();
}
return abbreviations.stream().filter(abbreviation -> isMatched(journalName.trim(), abbreviation)).findFirst();
}

public void addEntry(Abbreviation abbreviation) {
Objects.requireNonNull(abbreviation);

if (isKnownName(abbreviation.getName())) {
if (abbreviations.contains(abbreviation)) {
Abbreviation previous = getAbbreviation(abbreviation.getName()).get();
abbreviations.remove(previous);
LOGGER.info("Duplicate journal abbreviation - old one will be overwritten by new one\nOLD: "
+ previous + "\nNEW: " + abbreviation);
}

abbreviations.add(abbreviation);

fullNameLowerCase2Abbreviation.put(abbreviation.getName().toLowerCase(Locale.ENGLISH), abbreviation);
isoLowerCase2Abbreviation.put(abbreviation.getIsoAbbreviation().toLowerCase(Locale.ENGLISH), abbreviation);
medlineLowerCase2Abbreviation.put(abbreviation.getMedlineAbbreviation().toLowerCase(Locale.ENGLISH),
abbreviation);
}

public void addEntries(List<Abbreviation> abbreviationsToAdd) {
public void addEntries(Collection<Abbreviation> abbreviationsToAdd) {
abbreviationsToAdd.forEach(this::addEntry);
}

public SortedSet<Abbreviation> getAbbreviations() {
return Collections.unmodifiableSortedSet(abbreviations);
public Set<Abbreviation> getAbbreviations() {
return Collections.unmodifiableSet(abbreviations);
}

public Optional<String> getNextAbbreviation(String text) {
Optional<Abbreviation> abbreviation = getAbbreviation(text);

if (!abbreviation.isPresent()) {
return Optional.empty();
}

Abbreviation abbr = abbreviation.get();
return Optional.of(abbr.getNext(text));
return getAbbreviation(text).map(abbreviation -> abbreviation.getNext(text));
}

public Optional<String> getMedlineAbbreviation(String text) {
Optional<Abbreviation> abbreviation = getAbbreviation(text);

if (!abbreviation.isPresent()) {
return Optional.empty();
}

Abbreviation abbr = abbreviation.get();
return Optional.of(abbr.getMedlineAbbreviation());
return getAbbreviation(text).map(Abbreviation::getMedlineAbbreviation);
}

public Optional<String> getIsoAbbreviation(String text) {
Optional<Abbreviation> abbreviation = getAbbreviation(text);

if (!abbreviation.isPresent()) {
return Optional.empty();
}

Abbreviation abbr = abbreviation.get();
return Optional.of(abbr.getIsoAbbreviation());
return getAbbreviation(text).map(Abbreviation::getIsoAbbreviation);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ public void testSaveAbbreviationsToFilesCreatesNewFilesWithWrittenAbbreviations(

Assert.assertEquals(expected, actual);

expected = "Abbreviations = Abb" + NEWLINE + "Test Entry = TE" + NEWLINE + "MoreEntries = ME" + NEWLINE
expected = "EntryEntry = EE" + NEWLINE + "Abbreviations = Abb" + NEWLINE + "Test Entry = TE" + NEWLINE
+ "SomeOtherEntry = SOE" + NEWLINE + "";
actual = Files.contentOf(testFile5EntriesWithDuplicate.toFile(), StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,6 @@ public void oneElement() {

}

@Test
public void testSorting() {
JournalAbbreviationRepository repository = new JournalAbbreviationRepository();
repository.addEntry(new Abbreviation("Long Name", "L. N."));
repository.addEntry(new Abbreviation("A Long Name", "AL. N."));
assertEquals("A Long Name", repository.getAbbreviations().first().getName());
assertEquals("Long Name", repository.getAbbreviations().last().getName());
}

@Test
public void testDuplicates() {
JournalAbbreviationRepository repository = new JournalAbbreviationRepository();
Expand All @@ -63,9 +54,6 @@ public void testDuplicatesIsoOnly() {
repository.addEntry(new Abbreviation("Old Long Name", "L. N."));
repository.addEntry(new Abbreviation("New Long Name", "L. N."));
assertEquals(2, repository.size());

assertEquals("L N", repository.getNextAbbreviation("L. N.").orElse("WRONG"));
assertEquals("New Long Name", repository.getNextAbbreviation("L N").orElse("WRONG"));
}

@Test
Expand Down