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 massively by fixing a stupid binding mistake #6316

Merged
merged 1 commit into from
Apr 20, 2020
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
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