Skip to content

Commit

Permalink
Improve performance (#6270)
Browse files Browse the repository at this point in the history
* Improve performance

- Cache latex free field values for display in the main table
- Only access preferences once for how to display names (instead of for every entry again)
- Update search async only after 400ms without typing

* Fix tests
  • Loading branch information
tobiasdiez committed Apr 12, 2020
1 parent 1c724f2 commit 771af5d
Show file tree
Hide file tree
Showing 10 changed files with 220 additions and 259 deletions.
3 changes: 3 additions & 0 deletions .idea/runConfigurations/JabRef_Main.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,7 @@ private void createMainTable() {
mainTable.addSelectionListener(event -> mainTable.getSelectedEntries()
.stream()
.findFirst()
.ifPresent(entry -> {
entryEditor.setEntry(entry);
}));
.ifPresent(entryEditor::setEntry));

// TODO: Register these actions globally
/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public BibEntry getEntry() {
}

public Optional<String> getResolvedFieldOrAlias(Field field, BibDatabase database) {
return entry.getResolvedFieldOrAlias(field, database);
return entry.getResolvedFieldOrAliasLatexFree(field, database);
}

public ObjectBinding<String> getField(Field field) {
Expand Down
55 changes: 7 additions & 48 deletions src/main/java/org/jabref/gui/maintable/FieldColumn.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@
import javafx.beans.binding.ObjectBinding;
import javafx.beans.value.ObservableValue;

import org.jabref.logic.layout.LayoutFormatter;
import org.jabref.logic.layout.format.LatexToUnicodeFormatter;
import org.jabref.Globals;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldProperty;
import org.jabref.model.entry.field.InternalField;
import org.jabref.model.entry.field.OrFields;

/**
Expand All @@ -24,12 +21,13 @@ public class FieldColumn extends MainTableColumn<String> {

private final Optional<BibDatabase> database;

private final LayoutFormatter toUnicode = new LatexToUnicodeFormatter();
private final MainTableNameFormatter nameFormatter;

public FieldColumn(MainTableColumnModel model, OrFields bibtexFields, BibDatabase database) {
super(model);
this.bibtexFields = bibtexFields;
this.database = Optional.of(database);
this.nameFormatter = new MainTableNameFormatter(Globals.prefs);

setText(getDisplayName());
setCellValueFactory(param -> getColumnValue(param.getValue()));
Expand All @@ -43,7 +41,7 @@ public FieldColumn(MainTableColumnModel model, OrFields bibtexFields, BibDatabas
@Override
public String getDisplayName() { return bibtexFields.getDisplayName(); }

public ObservableValue<String> getColumnValue(BibEntryTableViewModel entry) {
private ObservableValue<String> getColumnValue(BibEntryTableViewModel entry) {
if (bibtexFields.isEmpty()) {
return null;
}
Expand All @@ -67,48 +65,9 @@ private String computeText(BibEntryTableViewModel entry) {
String result = content.orElse(null);

if (isNameColumn) {
result = toUnicode.format(MainTableNameFormatter.formatName(result));
return nameFormatter.formatName(result);
} else {
return result;
}

if ((result != null) && !bibtexFields.contains(InternalField.KEY_FIELD)) {
result = toUnicode.format(result).trim();
}
return result;
}

/**
* Check if the value returned by getColumnValue() is the same as a simple check of the entry's field(s) would give
* The reasons for being different are (combinations may also happen): - The entry has a crossref where the field
* content is obtained from - The field has a string in it (which getColumnValue() resolves) - There are some alias
* fields. For example, if the entry has a date field but no year field, {@link
* BibEntry#getResolvedFieldOrAlias(Field, BibDatabase)} will return the year value from the date field when
* queried for year
*
* @param entry the BibEntry
* @return true if the value returned by getColumnValue() is resolved as outlined above
*/
public boolean isResolved(BibEntry entry) {
if (bibtexFields.isEmpty()) {
return false;
}

Optional<String> resolvedFieldContent = Optional.empty();
Optional<String> plainFieldContent = Optional.empty();
for (Field field : bibtexFields) {
// entry type or bibtex key will never be resolved
if (InternalField.TYPE_HEADER.equals(field) || InternalField.OBSOLETE_TYPE_HEADER.equals(field)
|| InternalField.KEY_FIELD.equals(field)) {
return false;
} else {
plainFieldContent = entry.getField(field);
resolvedFieldContent = entry.getResolvedFieldOrAlias(field, database.orElse(null));
}

if (resolvedFieldContent.isPresent()) {
break;
}
}
return (!resolvedFieldContent.equals(plainFieldContent));
}

}
31 changes: 16 additions & 15 deletions src/main/java/org/jabref/gui/maintable/MainTableNameFormatter.java
Original file line number Diff line number Diff line change
@@ -1,32 +1,35 @@
package org.jabref.gui.maintable;

import org.jabref.Globals;
import org.jabref.model.entry.AuthorList;
import org.jabref.preferences.JabRefPreferences;

public class MainTableNameFormatter {

private MainTableNameFormatter() { }
private final boolean namesNatbib;
private final boolean namesLastOnly;
private final boolean namesAsIs;
private final boolean namesFf;
private final boolean abbrAuthorNames;

MainTableNameFormatter(JabRefPreferences preferences) {
namesNatbib = preferences.getBoolean(JabRefPreferences.NAMES_NATBIB);
namesLastOnly = preferences.getBoolean(JabRefPreferences.NAMES_LAST_ONLY);
namesAsIs = preferences.getBoolean(JabRefPreferences.NAMES_AS_IS);
namesFf = preferences.getBoolean(JabRefPreferences.NAMES_FIRST_LAST);
abbrAuthorNames = preferences.getBoolean(JabRefPreferences.ABBR_AUTHOR_NAMES);
}

/**
* Format a name field for the table, according to user preferences.
*
* @param nameToFormat The contents of the name field.
* @return The formatted name field.
*/
public static String formatName(final String nameToFormat) {
public String formatName(final String nameToFormat) {
if (nameToFormat == null) {
return null;
}

// Read name format options:
final boolean namesNatbib = Globals.prefs.getBoolean(JabRefPreferences.NAMES_NATBIB); //MK:
final boolean namesLastOnly = Globals.prefs.getBoolean(JabRefPreferences.NAMES_LAST_ONLY);
final boolean namesAsIs = Globals.prefs.getBoolean(JabRefPreferences.NAMES_AS_IS);
final boolean namesFf = Globals.prefs.getBoolean(JabRefPreferences.NAMES_FIRST_LAST);

final boolean abbrAuthorNames = Globals.prefs.getBoolean(JabRefPreferences.ABBR_AUTHOR_NAMES); //MK:

if (namesAsIs) {
return nameToFormat;
} else if (namesNatbib) {
Expand All @@ -35,10 +38,8 @@ public static String formatName(final String nameToFormat) {
return AuthorList.fixAuthorLastNameOnlyCommas(nameToFormat, false);
} else if (namesFf) {
return AuthorList.fixAuthorFirstNameFirstCommas(nameToFormat, abbrAuthorNames, false);
} else {
return AuthorList.fixAuthorLastNameFirstCommas(nameToFormat, abbrAuthorNames, false);
}

// None of namesAsIs, namesNatbib, namesAsIs, namesFf
return AuthorList.fixAuthorLastNameFirstCommas(nameToFormat, abbrAuthorNames, false);
}

}
9 changes: 4 additions & 5 deletions src/main/java/org/jabref/gui/search/GlobalSearchBar.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public GlobalSearchBar(JabRefFrame frame, StateManager stateManager) {

SearchPreferences searchPreferences = new SearchPreferences(Globals.prefs);
searchDisplayMode = searchPreferences.getSearchMode();

this.searchField.disableProperty().bind(needsDatabase(stateManager).not());

// fits the standard "found x entries"-message thus hinders the searchbar to jump around while searching if the frame width is too small
Expand Down Expand Up @@ -162,9 +162,6 @@ public GlobalSearchBar(JabRefFrame frame, StateManager stateManager) {
visualizer.setDecoration(new IconValidationDecorator(Pos.CENTER_LEFT));
Platform.runLater(() -> { visualizer.initVisualization(regexValidator.getValidationStatus(), searchField); });

Timer searchTask = FxTimer.create(java.time.Duration.ofMillis(SEARCH_DELAY), this::performSearch);
searchField.textProperty().addListener((observable, oldValue, newValue) -> searchTask.restart());

EasyBind.subscribe(searchField.focusedProperty(), isFocused -> {
if (isFocused) {
KeyValue widthValue = new KeyValue(searchField.maxWidthProperty(), expandedSize);
Expand All @@ -183,11 +180,13 @@ public GlobalSearchBar(JabRefFrame frame, StateManager stateManager) {

this.setAlignment(Pos.CENTER_LEFT);

Timer searchTask = FxTimer.create(java.time.Duration.ofMillis(SEARCH_DELAY), this::performSearch);
BindingsHelper.bindBidirectional(
stateManager.activeSearchQueryProperty(),
searchField.textProperty(),
searchTerm -> {
performSearch();
// Async update
searchTask.restart();
},
query -> setSearchTerm(query.map(SearchQuery::getQuery).orElse(""))
);
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/org/jabref/gui/util/ValueTableCellFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import javafx.scene.control.ContextMenu;
import javafx.scene.control.TableCell;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableRow;
import javafx.scene.control.Tooltip;
import javafx.scene.input.MouseButton;
import javafx.scene.input.MouseEvent;
Expand Down Expand Up @@ -78,8 +77,7 @@ public ValueTableCellFactory<S, T> withMenu(BiFunction<S, T, ContextMenu> menuFa

@Override
public TableCell<S, T> call(TableColumn<S, T> param) {

return new TableCell<S, T>() {
return new TableCell<>() {

@Override
protected void updateItem(T item, boolean empty) {
Expand All @@ -91,7 +89,7 @@ protected void updateItem(T item, boolean empty) {
setOnMouseClicked(null);
setTooltip(null);
} else {
S rowItem = ((TableRow<S>) getTableRow()).getItem();
S rowItem = getTableRow().getItem();

if (toText != null) {
setText(toText.apply(item));
Expand Down
51 changes: 27 additions & 24 deletions src/main/java/org/jabref/model/entry/BibEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BiFunction;
import java.util.regex.Pattern;

import javafx.beans.Observable;
Expand Down Expand Up @@ -247,6 +248,14 @@ private Optional<Field> getSourceField(Field targetField, EntryType targetEntry,
* @return The resolved field value or null if not found.
*/
public Optional<String> getResolvedFieldOrAlias(Field field, BibDatabase database) {
return genericGetResolvedFieldOrAlias(field, database, BibEntry::getFieldOrAlias);
}

public Optional<String> getResolvedFieldOrAliasLatexFree(Field field, BibDatabase database) {
return genericGetResolvedFieldOrAlias(field, database, BibEntry::getFieldOrAliasLatexFree);
}

private Optional<String> genericGetResolvedFieldOrAlias(Field field, BibDatabase database, BiFunction<BibEntry, Field, Optional<String>> getFieldOrAlias) {
if (InternalField.TYPE_HEADER.equals(field) || InternalField.OBSOLETE_TYPE_HEADER.equals(field)) {
return Optional.of(type.get().getDisplayName());
}
Expand All @@ -255,7 +264,7 @@ public Optional<String> getResolvedFieldOrAlias(Field field, BibDatabase databas
return getCiteKeyOptional();
}

Optional<String> result = getFieldOrAlias(field);
Optional<String> result = getFieldOrAlias.apply(this, field);
// If this field is not set, and the entry has a crossref, try to look up the
// field in the referred entry, following the biblatex rules
if (result.isEmpty() && (database != null)) {
Expand All @@ -266,7 +275,7 @@ public Optional<String> getResolvedFieldOrAlias(Field field, BibDatabase databas
Optional<Field> sourceField = getSourceField(field, targetEntry, sourceEntry);

if (sourceField.isPresent()) {
result = referred.get().getFieldOrAlias(sourceField.get());
result = getFieldOrAlias.apply(referred.get(), sourceField.get());
}
}
}
Expand All @@ -281,7 +290,7 @@ public String getId() {
}

/**
* Sets this entry's identifier (ID). It is used internally to distinguish different BibTeX entries. It is <emph>not</emph> the BibTeX key. The BibTexKey is the {@link InternalField.KEY_FIELD}.
* Sets this entry's identifier (ID). It is used internally to distinguish different BibTeX entries. It is <emph>not</emph> the BibTeX key. The BibTexKey is the {@link InternalField#KEY_FIELD}.
*
* The entry is also updated in the shared database - provided the database containing it doesn't veto the change.
*
Expand Down Expand Up @@ -396,13 +405,12 @@ public boolean hasField(Field field) {
*
* Used by {@link #getFieldOrAlias(Field)} and {@link #getFieldOrAliasLatexFree(Field)}
*
* @param field the field
* @param getFieldInterface
*
* @param field the field
* @param getFieldValue the method to get the value of a given field in a given entry
* @return determined field value
*/
private Optional<String> genericGetFieldOrAlias(Field field, GetFieldInterface getFieldInterface) {
Optional<String> fieldValue = getFieldInterface.getValueForField(field);
private Optional<String> genericGetFieldOrAlias(Field field, BiFunction<BibEntry, Field, Optional<String>> getFieldValue) {
Optional<String> fieldValue = getFieldValue.apply(this, field);

if (fieldValue.isPresent() && !fieldValue.get().isEmpty()) {
return fieldValue;
Expand All @@ -412,22 +420,22 @@ private Optional<String> genericGetFieldOrAlias(Field field, GetFieldInterface g
Field aliasForField = EntryConverter.FIELD_ALIASES.get(field);

if (aliasForField != null) {
return getFieldInterface.getValueForField(aliasForField);
return getFieldValue.apply(this, aliasForField);
}

// Finally, handle dates
if (StandardField.DATE.equals(field)) {
Optional<Date> date = Date.parse(
getFieldInterface.getValueForField(StandardField.YEAR),
getFieldInterface.getValueForField(StandardField.MONTH),
getFieldInterface.getValueForField(StandardField.DAY));
getFieldValue.apply(this, StandardField.YEAR),
getFieldValue.apply(this, StandardField.MONTH),
getFieldValue.apply(this, StandardField.DAY));

return date.map(Date::getNormalized);
}

if (StandardField.YEAR.equals(field) || StandardField.MONTH.equals(field) || StandardField.DAY.equals(field)) {
Optional<String> date = getFieldInterface.getValueForField(StandardField.DATE);
if (!date.isPresent()) {
Optional<String> date = getFieldValue.apply(this, StandardField.DATE);
if (date.isEmpty()) {
return Optional.empty();
}

Expand Down Expand Up @@ -464,7 +472,7 @@ public Optional<DOI> getDOI() {
* @return the stored latex-free content of the field (or its alias)
*/
public Optional<String> getFieldOrAliasLatexFree(Field name) {
return genericGetFieldOrAlias(name, this::getLatexFreeField);
return genericGetFieldOrAlias(name, BibEntry::getLatexFreeField);
}

/**
Expand Down Expand Up @@ -492,7 +500,7 @@ public Optional<String> getFieldOrAliasLatexFree(Field name) {
* </p>
*/
public Optional<String> getFieldOrAlias(Field field) {
return genericGetFieldOrAlias(field, this::getField);
return genericGetFieldOrAlias(field, BibEntry::getField);
}

/**
Expand Down Expand Up @@ -870,9 +878,9 @@ public Optional<String> getLatexFreeField(Field field) {
} else {
Optional<String> fieldValue = getField(field);
if (fieldValue.isPresent()) {
String latexFreeField = LatexToUnicodeAdapter.format(fieldValue.get()).intern();
latexFreeFields.put(field, latexFreeField);
return Optional.of(latexFreeField);
String latexFreeValue = LatexToUnicodeAdapter.format(fieldValue.get()).intern();
latexFreeFields.put(field, latexFreeValue);
return Optional.of(latexFreeValue);
} else {
return Optional.empty();
}
Expand Down Expand Up @@ -943,9 +951,4 @@ public ObservableMap<Field, String> getFieldsObservable() {
public Observable[] getObservables() {
return new Observable[] {fields, type};
}

private interface GetFieldInterface {
Optional<String> getValueForField(Field field);
}

}
Loading

0 comments on commit 771af5d

Please sign in to comment.