Skip to content

Commit

Permalink
Improve performance massively by fixing a stupid binding mistake (#6316)
Browse files Browse the repository at this point in the history
  • Loading branch information
tobiasdiez committed Apr 20, 2020
1 parent 5ed32e1 commit bc57d22
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 68 deletions.
97 changes: 77 additions & 20 deletions src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,64 +12,92 @@
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.value.ObservableValue;

import org.jabref.Globals;
import org.jabref.gui.specialfields.SpecialFieldValueViewModel;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.FileFieldParser;
import org.jabref.model.entry.LinkedFile;
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;
import org.jabref.model.entry.field.SpecialField;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.groups.AbstractGroup;
import org.jabref.model.groups.GroupTreeNode;

import org.fxmisc.easybind.EasyBind;
import org.fxmisc.easybind.monadic.MonadicBinding;

public class BibEntryTableViewModel {
private final BibEntry entry;
private final BibDatabase database;
private final MainTableNameFormatter nameFormatter;
private final Map<OrFields, ObservableValue<String>> fieldValues = new HashMap<>();
private final Map<SpecialField, ObservableValue<Optional<SpecialFieldValueViewModel>>> specialFieldValues = new HashMap<>();
private final MonadicBinding<List<LinkedFile>> linkedFiles;
private final ObjectBinding<Map<Field, String>> linkedIdentifiers;
private final ObservableValue<List<AbstractGroup>> matchedGroups;

public BibEntryTableViewModel(BibEntry entry) {
public BibEntryTableViewModel(BibEntry entry, BibDatabaseContext database) {
this.entry = entry;
this.database = database.getDatabase();
this.nameFormatter = new MainTableNameFormatter(Globals.prefs);

this.linkedFiles = EasyBind.map(getField(StandardField.FILE), FileFieldParser::parse);
this.linkedIdentifiers = createLinkedIdentifiersBinding(entry);
this.matchedGroups = createMatchedGroupsBinding(database);
}

public BibEntry getEntry() {
return entry;
private ObjectBinding<Map<Field, String>> createLinkedIdentifiersBinding(BibEntry entry) {
return Bindings.createObjectBinding(() -> {
Map<Field, String> identifiers = new HashMap<>();
entry.getField(StandardField.URL).ifPresent(value -> identifiers.put(StandardField.URL, value));
entry.getField(StandardField.DOI).ifPresent(value -> identifiers.put(StandardField.DOI, value));
entry.getField(StandardField.URI).ifPresent(value -> identifiers.put(StandardField.URI, value));
entry.getField(StandardField.EPRINT).ifPresent(value -> identifiers.put(StandardField.EPRINT, value));
return identifiers;
},
getEntry().getFieldBinding(StandardField.URL),
getEntry().getFieldBinding(StandardField.DOI),
getEntry().getFieldBinding(StandardField.URI),
getEntry().getFieldBinding(StandardField.EPRINT));
}

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

public ObjectBinding<String> getField(Field field) {
return entry.getFieldBinding(field);
}

public ObservableValue<Optional<SpecialFieldValueViewModel>> getSpecialField(SpecialField field) {
return EasyBind.map(getField(field), value -> field.parseValue(value).map(SpecialFieldValueViewModel::new));
ObservableValue<Optional<SpecialFieldValueViewModel>> value = specialFieldValues.get(field);
if (value != null) {
return value;
} else {
value = EasyBind.map(getField(field), fieldValue -> field.parseValue(fieldValue).map(SpecialFieldValueViewModel::new));
specialFieldValues.put(field, value);
return value;
}
}

public ObservableValue<List<LinkedFile>> getLinkedFiles() {
return EasyBind.map(getField(StandardField.FILE), FileFieldParser::parse);
return linkedFiles;
}

public ObservableValue<Map<Field, String>> getLinkedIdentifiers() {
return Bindings.createObjectBinding(() -> {
Map<Field, String> linkedIdentifiers = new HashMap<>();
entry.getField(StandardField.URL).ifPresent(value -> linkedIdentifiers.put(StandardField.URL, value));
entry.getField(StandardField.DOI).ifPresent(value -> linkedIdentifiers.put(StandardField.DOI, value));
entry.getField(StandardField.URI).ifPresent(value -> linkedIdentifiers.put(StandardField.URI, value));
entry.getField(StandardField.EPRINT).ifPresent(value -> linkedIdentifiers.put(StandardField.EPRINT, value));
return linkedIdentifiers;
},
getEntry().getFieldBinding(StandardField.URL),
getEntry().getFieldBinding(StandardField.DOI),
getEntry().getFieldBinding(StandardField.URI),
getEntry().getFieldBinding(StandardField.EPRINT));
return linkedIdentifiers;
}

public ObservableValue<List<AbstractGroup>> getMatchedGroups() {
return matchedGroups;
}

public ObservableValue<List<AbstractGroup>> getMatchedGroups(BibDatabaseContext database) {
private ObservableValue<List<AbstractGroup>> createMatchedGroupsBinding(BibDatabaseContext database) {
Optional<GroupTreeNode> root = database.getMetaData().getGroups();
if (root.isPresent()) {
return EasyBind.map(entry.getFieldBinding(InternalField.GROUPS), field -> {
Expand All @@ -83,4 +111,33 @@ public ObservableValue<List<AbstractGroup>> getMatchedGroups(BibDatabaseContext
}
return new SimpleObjectProperty<>(Collections.emptyList());
}

public ObservableValue<String> getFields(OrFields fields) {
ObservableValue<String> value = fieldValues.get(fields);
if (value != null) {
return value;
} else {
value = Bindings.createStringBinding(() -> {
boolean isName = false;

Optional<String> content = Optional.empty();
for (Field field : fields) {
content = entry.getResolvedFieldOrAlias(field, database);
if (content.isPresent()) {
isName = field.getProperties().contains(FieldProperty.PERSON_NAMES);
break;
}
}

String result = content.orElse(null);
if (isName) {
return nameFormatter.formatName(result);
} else {
return result;
}
}, entry.getObservables());
fieldValues.put(fields, value);
return value;
}
}
}
54 changes: 10 additions & 44 deletions src/main/java/org/jabref/gui/maintable/FieldColumn.java
Original file line number Diff line number Diff line change
@@ -1,36 +1,22 @@
package org.jabref.gui.maintable;

import java.util.Optional;

import javafx.beans.binding.Bindings;
import javafx.beans.binding.ObjectBinding;
import javafx.beans.value.ObservableValue;

import org.jabref.Globals;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldProperty;
import org.jabref.model.entry.field.OrFields;

/**
* A column that displays the text-value of the field
*/
public class FieldColumn extends MainTableColumn<String> {

private final OrFields bibtexFields;

private final Optional<BibDatabase> database;

private final MainTableNameFormatter nameFormatter;
private final OrFields fields;

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

setText(getDisplayName());
setCellValueFactory(param -> getColumnValue(param.getValue()));
setCellValueFactory(param -> getFieldValue(param.getValue()));
}

/**
Expand All @@ -39,35 +25,15 @@ public FieldColumn(MainTableColumnModel model, OrFields bibtexFields, BibDatabas
* @return name to be displayed. null if field is empty.
*/
@Override
public String getDisplayName() { return bibtexFields.getDisplayName(); }

private ObservableValue<String> getColumnValue(BibEntryTableViewModel entry) {
if (bibtexFields.isEmpty()) {
return null;
}

ObjectBinding[] dependencies = bibtexFields.stream().map(entry::getField).toArray(ObjectBinding[]::new);
return Bindings.createStringBinding(() -> computeText(entry), dependencies);
public String getDisplayName() {
return fields.getDisplayName();
}

private String computeText(BibEntryTableViewModel entry) {
boolean isNameColumn = false;

Optional<String> content = Optional.empty();
for (Field field : bibtexFields) {
content = entry.getResolvedFieldOrAlias(field, database.orElse(null));
if (content.isPresent()) {
isNameColumn = field.getProperties().contains(FieldProperty.PERSON_NAMES);
break;
}
}

String result = content.orElse(null);

if (isNameColumn) {
return nameFormatter.formatName(result);
private ObservableValue<String> getFieldValue(BibEntryTableViewModel entry) {
if (fields.isEmpty()) {
return null;
} else {
return result;
return entry.getFields(fields);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ private TableColumn<BibEntryTableViewModel, String> createIndexColumn(MainTableC
column.getStyleClass().add(STYLE_ICON_COLUMN);
setExactWidth(column, ColumnPreferences.ICON_COLUMN_WIDTH);
column.setResizable(false);
column.setCellValueFactory(cellData -> cellData.getValue().getMatchedGroups(database));
column.setCellValueFactory(cellData -> cellData.getValue().getMatchedGroups());
new ValueTableCellFactory<BibEntryTableViewModel, List<AbstractGroup>>()
.withGraphic(this::createGroupColorRegion)
.install(column);
Expand Down Expand Up @@ -207,8 +207,8 @@ private Node createGroupColorRegion(BibEntryTableViewModel entry, List<AbstractG
*/
private TableColumn<BibEntryTableViewModel, ?> createFieldColumn(MainTableColumnModel columnModel) {
FieldColumn column = new FieldColumn(columnModel,
FieldFactory.parseOrFields(columnModel.getQualifier()),
database.getDatabase());
FieldFactory.parseOrFields(columnModel.getQualifier())
);
new ValueTableCellFactory<BibEntryTableViewModel, String>()
.withText(text -> text)
.install(column);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class MainTableDataModel {
public MainTableDataModel(BibDatabaseContext context) {
ObservableList<BibEntry> allEntries = BindingsHelper.forUI(context.getDatabase().getEntries());

ObservableList<BibEntryTableViewModel> entriesViewModel = BindingsHelper.mapBacked(allEntries, BibEntryTableViewModel::new);
ObservableList<BibEntryTableViewModel> entriesViewModel = BindingsHelper.mapBacked(allEntries, entry -> new BibEntryTableViewModel(entry, context));

entriesFiltered = new FilteredList<>(entriesViewModel);
entriesFiltered.predicateProperty().bind(
Expand Down

0 comments on commit bc57d22

Please sign in to comment.