From bc57d22f2a4ae8912d263608cf0a606fbbecd43a Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Mon, 20 Apr 2020 08:24:58 +0200 Subject: [PATCH] Improve performance massively by fixing a stupid binding mistake (#6316) --- .../gui/maintable/BibEntryTableViewModel.java | 97 +++++++++++++++---- .../org/jabref/gui/maintable/FieldColumn.java | 54 ++--------- .../gui/maintable/MainTableColumnFactory.java | 6 +- .../gui/maintable/MainTableDataModel.java | 2 +- 4 files changed, 91 insertions(+), 68 deletions(-) diff --git a/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java b/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java index 9397f26de16..8ef28292285 100644 --- a/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java +++ b/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java @@ -12,6 +12,7 @@ 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; @@ -19,27 +20,54 @@ 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> fieldValues = new HashMap<>(); + private final Map>> specialFieldValues = new HashMap<>(); + private final MonadicBinding> linkedFiles; + private final ObjectBinding> linkedIdentifiers; + private final ObservableValue> 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> createLinkedIdentifiersBinding(BibEntry entry) { + return Bindings.createObjectBinding(() -> { + Map 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 getResolvedFieldOrAlias(Field field, BibDatabase database) { - return entry.getResolvedFieldOrAliasLatexFree(field, database); + public BibEntry getEntry() { + return entry; } public ObjectBinding getField(Field field) { @@ -47,29 +75,29 @@ public ObjectBinding getField(Field field) { } public ObservableValue> getSpecialField(SpecialField field) { - return EasyBind.map(getField(field), value -> field.parseValue(value).map(SpecialFieldValueViewModel::new)); + ObservableValue> 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> getLinkedFiles() { - return EasyBind.map(getField(StandardField.FILE), FileFieldParser::parse); + return linkedFiles; } public ObservableValue> getLinkedIdentifiers() { - return Bindings.createObjectBinding(() -> { - Map 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> getMatchedGroups() { + return matchedGroups; } - public ObservableValue> getMatchedGroups(BibDatabaseContext database) { + private ObservableValue> createMatchedGroupsBinding(BibDatabaseContext database) { Optional root = database.getMetaData().getGroups(); if (root.isPresent()) { return EasyBind.map(entry.getFieldBinding(InternalField.GROUPS), field -> { @@ -83,4 +111,33 @@ public ObservableValue> getMatchedGroups(BibDatabaseContext } return new SimpleObjectProperty<>(Collections.emptyList()); } + + public ObservableValue getFields(OrFields fields) { + ObservableValue value = fieldValues.get(fields); + if (value != null) { + return value; + } else { + value = Bindings.createStringBinding(() -> { + boolean isName = false; + + Optional 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; + } + } } diff --git a/src/main/java/org/jabref/gui/maintable/FieldColumn.java b/src/main/java/org/jabref/gui/maintable/FieldColumn.java index cc221fcf722..fac40ce2d87 100644 --- a/src/main/java/org/jabref/gui/maintable/FieldColumn.java +++ b/src/main/java/org/jabref/gui/maintable/FieldColumn.java @@ -1,15 +1,7 @@ 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; /** @@ -17,20 +9,14 @@ */ public class FieldColumn extends MainTableColumn { - private final OrFields bibtexFields; - - private final Optional 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())); } /** @@ -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 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 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 getFieldValue(BibEntryTableViewModel entry) { + if (fields.isEmpty()) { + return null; } else { - return result; + return entry.getFields(fields); } } } diff --git a/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java b/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java index a821e343d01..2327da419a2 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java +++ b/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java @@ -160,7 +160,7 @@ private TableColumn 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>() .withGraphic(this::createGroupColorRegion) .install(column); @@ -207,8 +207,8 @@ private Node createGroupColorRegion(BibEntryTableViewModel entry, List createFieldColumn(MainTableColumnModel columnModel) { FieldColumn column = new FieldColumn(columnModel, - FieldFactory.parseOrFields(columnModel.getQualifier()), - database.getDatabase()); + FieldFactory.parseOrFields(columnModel.getQualifier()) + ); new ValueTableCellFactory() .withText(text -> text) .install(column); diff --git a/src/main/java/org/jabref/gui/maintable/MainTableDataModel.java b/src/main/java/org/jabref/gui/maintable/MainTableDataModel.java index 192dd715f4f..c0ee007455b 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTableDataModel.java +++ b/src/main/java/org/jabref/gui/maintable/MainTableDataModel.java @@ -27,7 +27,7 @@ public class MainTableDataModel { public MainTableDataModel(BibDatabaseContext context) { ObservableList allEntries = BindingsHelper.forUI(context.getDatabase().getEntries()); - ObservableList entriesViewModel = BindingsHelper.mapBacked(allEntries, BibEntryTableViewModel::new); + ObservableList entriesViewModel = BindingsHelper.mapBacked(allEntries, entry -> new BibEntryTableViewModel(entry, context)); entriesFiltered = new FilteredList<>(entriesViewModel); entriesFiltered.predicateProperty().bind(