Skip to content

Commit

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

- Performance improvements around groups
- Remove detection of duplicate ID when inserting entries (it's not really possible to create two entries with the same except if you use `setId` manually)
- Remove detection of duplicate bibtex keys when opening a file (the result was not used and we have a integrity check for it)
- Use EnumMap instead of HashMap to cache fields as keywords (which is only used for Keyword and Groups fields anyway)
- Fix bug where latex code was displayed in the maintable
- Lazy init of source tab

* Fix tests compilation

* Include feedback
  • Loading branch information
tobiasdiez authored Apr 22, 2020
1 parent 17a97fd commit c7d7767
Show file tree
Hide file tree
Showing 39 changed files with 309 additions and 326 deletions.
1 change: 1 addition & 0 deletions .idea/runConfigurations/JabRef_Main.xml

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

5 changes: 0 additions & 5 deletions src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.jabref.model.FieldChange;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.KeyCollisionException;
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
import org.jabref.model.database.event.EntriesAddedEvent;
import org.jabref.model.database.event.EntriesRemovedEvent;
Expand Down Expand Up @@ -253,7 +252,6 @@ public void insertEntry(final BibEntry bibEntry) {

public void insertEntries(final List<BibEntry> entries) {
if (!entries.isEmpty()) {
try {
bibDatabaseContext.getDatabase().insertEntries(entries);

// Set owner and timestamp
Expand All @@ -272,9 +270,6 @@ public void insertEntries(final List<BibEntry> entries) {
showAndEdit(entries.get(0));
}
clearAndSelect(entries.get(0));
} catch (KeyCollisionException ex) {
LOGGER.info("Collision for bibtex key" + ex.getId(), ex);
}
}
}

Expand Down
24 changes: 14 additions & 10 deletions src/main/java/org/jabref/gui/entryeditor/SourceTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,18 @@ public class SourceTab extends EntryEditorTab {
private final DialogService dialogService;
private final StateManager stateManager;
private Optional<Pattern> searchHighlightPattern = Optional.empty();
private final CodeArea codeArea;
private KeyBindingRepository keyBindingRepository;
private final KeyBindingRepository keyBindingRepository;
private CodeArea codeArea;

private BibEntry previousEntry;

private class EditAction extends SimpleCommand {

private final StandardActions command;

public EditAction(StandardActions command) { this.command = command; }
public EditAction(StandardActions command) {
this.command = command;
}

@Override
public void execute() {
Expand Down Expand Up @@ -112,11 +114,6 @@ public SourceTab(BibDatabaseContext bibDatabaseContext, CountingUndoManager undo
this.dialogService = dialogService;
this.stateManager = stateManager;
this.keyBindingRepository = keyBindingRepository;
this.codeArea = new CodeArea();

setupSourceEditor();
VirtualizedScrollPane<CodeArea> scrollableCodeArea = new VirtualizedScrollPane<>(codeArea);
this.setContent(scrollableCodeArea);

stateManager.activeSearchQueryProperty().addListener((observable, oldValue, newValue) -> {
searchHighlightPattern = newValue.flatMap(SearchQuery::getPatternForWords);
Expand Down Expand Up @@ -172,6 +169,7 @@ public Point2D getTextLocation(int offset) {
}

private void setupSourceEditor() {
codeArea = new CodeArea();
codeArea.setWrapText(true);
codeArea.setInputMethodRequests(new InputMethodRequestsObject());
codeArea.setOnInputMethodTextChanged(event -> {
Expand Down Expand Up @@ -200,7 +198,7 @@ private void setupSourceEditor() {
ValidationStatus sourceValidationStatus = sourceValidator.getValidationStatus();
if (!sourceValidationStatus.isValid()) {
sourceValidationStatus.getHighestMessage().ifPresent(message ->
dialogService.showErrorDialogAndWait(message.getMessage()));
dialogService.showErrorDialogAndWait(message.getMessage()));
}
});

Expand All @@ -209,6 +207,8 @@ private void setupSourceEditor() {
storeSource(currentEntry, codeArea.textProperty().getValue());
}
});
VirtualizedScrollPane<CodeArea> scrollableCodeArea = new VirtualizedScrollPane<>(codeArea);
this.setContent(scrollableCodeArea);
}

@Override
Expand All @@ -218,6 +218,10 @@ public boolean shouldShow(BibEntry entry) {

private void updateCodeArea() {
DefaultTaskExecutor.runAndWaitInJavaFXThread(() -> {
if (codeArea == null) {
setupSourceEditor();
}

codeArea.clear();
try {
codeArea.appendText(getSourceString(currentEntry, mode, fieldWriterPreferences));
Expand All @@ -233,7 +237,7 @@ private void updateCodeArea() {

@Override
protected void bindToEntry(BibEntry entry) {
if (previousEntry != null) {
if (previousEntry != null && codeArea != null) {
storeSource(previousEntry, codeArea.textProperty().getValue());
}
this.previousEntry = entry;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public static FieldEditorFX getForField(final Field field,
return new JournalEditor(field, journalAbbreviationRepository, preferences, suggestionProvider, fieldCheckers);
} else if (fieldProperties.contains(FieldProperty.DOI) || fieldProperties.contains(FieldProperty.EPRINT) || fieldProperties.contains(FieldProperty.ISBN)) {
return new IdentifierEditor(field, taskExecutor, dialogService, suggestionProvider, fieldCheckers, preferences);
} else if (field == InternalField.OWNER) {
} else if (field == StandardField.OWNER) {
return new OwnerEditor(field, preferences, suggestionProvider, fieldCheckers);
} else if (fieldProperties.contains(FieldProperty.FILE_EDITOR)) {
return new LinkedFilesEditor(field, dialogService, databaseContext, taskExecutor, suggestionProvider, fieldCheckers, preferences);
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/org/jabref/gui/fieldeditors/FieldNameLabel.java
Original file line number Diff line number Diff line change
Expand Up @@ -216,18 +216,18 @@ public String getDescription(Field field) {
return Localization.lang("Citation keys of other entries which have a relationship to this entry.");
case XREF:
return Localization.lang("This field is an alternative cross-referencing mechanism. It differs from \"Crossref\" in that the child entry will not inherit any data from the parent entry specified in the \"Xref\" field.");
}
} else if (field instanceof InternalField) {
InternalField internalField = (InternalField) field;
switch (internalField) {
case GROUPS:
return Localization.lang("Name(s) of the (manual) groups the entry belongs to.");
case OWNER:
return Localization.lang("Owner/creator of this entry.");
case TIMESTAMP:
return Localization.lang("Timestamp of this entry, when it has been created or last modified.");
}
} else if (field instanceof InternalField) {
InternalField internalField = (InternalField) field;
switch (internalField) {
case KEY_FIELD:
return Localization.lang("Key by which the work may be cited.");
case GROUPS:
return Localization.lang("Name(s) of the (manual) groups the entry belongs to.");
}
} else if (field instanceof SpecialField) {
SpecialField specialField = (SpecialField) field;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
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;
Expand All @@ -21,7 +20,6 @@
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;
Expand All @@ -41,10 +39,10 @@ public class BibEntryTableViewModel {
private final ObjectBinding<Map<Field, String>> linkedIdentifiers;
private final ObservableValue<List<AbstractGroup>> matchedGroups;

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

this.linkedFiles = EasyBind.map(getField(StandardField.FILE), FileFieldParser::parse);
this.linkedIdentifiers = createLinkedIdentifiersBinding(entry);
Expand Down Expand Up @@ -100,7 +98,7 @@ public ObservableValue<List<AbstractGroup>> getMatchedGroups() {
private ObservableValue<List<AbstractGroup>> createMatchedGroupsBinding(BibDatabaseContext database) {
Optional<GroupTreeNode> root = database.getMetaData().getGroups();
if (root.isPresent()) {
return EasyBind.map(entry.getFieldBinding(InternalField.GROUPS), field -> {
return EasyBind.map(entry.getFieldBinding(StandardField.GROUPS), field -> {
List<AbstractGroup> groups = root.get().getMatchingGroups(entry)
.stream()
.map(GroupTreeNode::getGroup)
Expand All @@ -122,7 +120,7 @@ public ObservableValue<String> getFields(OrFields fields) {

Optional<String> content = Optional.empty();
for (Field field : fields) {
content = entry.getResolvedFieldOrAlias(field, database);
content = entry.getResolvedFieldOrAliasLatexFree(field, database);
if (content.isPresent()) {
isName = field.getProperties().contains(FieldProperty.PERSON_NAMES);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ public class MainTableDataModel {
public MainTableDataModel(BibDatabaseContext context) {
ObservableList<BibEntry> allEntries = BindingsHelper.forUI(context.getDatabase().getEntries());

ObservableList<BibEntryTableViewModel> entriesViewModel = BindingsHelper.mapBacked(allEntries, entry -> new BibEntryTableViewModel(entry, context));
MainTableNameFormatter nameFormatter = new MainTableNameFormatter(Globals.prefs);
ObservableList<BibEntryTableViewModel> entriesViewModel = BindingsHelper.mapBacked(allEntries, entry -> new BibEntryTableViewModel(entry, context, nameFormatter));

entriesFiltered = new FilteredList<>(entriesViewModel);
entriesFiltered.predicateProperty().bind(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ public MainTableColumnModel fromString(String string) {
private final BooleanProperty specialFieldsSerializeProperty = new SimpleBooleanProperty();
private final BooleanProperty extraFileColumnsEnabledProperty = new SimpleBooleanProperty();

private Validator columnsNotEmptyValidator;
private final Validator columnsNotEmptyValidator;

private List<String> restartWarnings = new ArrayList<>();
private final List<String> restartWarnings = new ArrayList<>();

private final DialogService dialogService;
private final JabRefPreferences preferences;
Expand Down Expand Up @@ -119,9 +119,9 @@ public void setValues() {
new MainTableColumnModel(MainTableColumnModel.Type.LINKED_IDENTIFIER),
new MainTableColumnModel(MainTableColumnModel.Type.GROUPS),
new MainTableColumnModel(MainTableColumnModel.Type.FILES),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, InternalField.TIMESTAMP.getName()),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, InternalField.OWNER.getName()),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, InternalField.GROUPS.getName()),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, StandardField.TIMESTAMP.getName()),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, StandardField.OWNER.getName()),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, StandardField.GROUPS.getName()),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, InternalField.KEY_FIELD.getName()),
new MainTableColumnModel(MainTableColumnModel.Type.NORMALFIELD, InternalField.TYPE_HEADER.getName())
);
Expand Down
32 changes: 1 addition & 31 deletions src/main/java/org/jabref/logic/importer/ParserResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
public class ParserResult {
private final Set<BibEntryType> entryTypes;
private final List<String> warnings = new ArrayList<>();
private final List<String> duplicateKeys = new ArrayList<>();
private BibDatabase database;
private MetaData metaData;
private Path file;
Expand All @@ -35,7 +34,7 @@ public ParserResult() {
}

public ParserResult(Collection<BibEntry> entries) {
this(BibDatabases.createDatabase(BibDatabases.purgeEmptyEntries(entries)));
this(new BibDatabase(BibDatabases.purgeEmptyEntries(entries)));
}

public ParserResult(BibDatabase database) {
Expand Down Expand Up @@ -126,35 +125,6 @@ public List<String> warnings() {
return new ArrayList<>(warnings);
}

/**
* Add a key to the list of duplicated BibTeX keys found in the database.
*
* @param key The duplicated key
*/
public void addDuplicateKey(String key) {
if (!duplicateKeys.contains(key)) {
duplicateKeys.add(key);
}
}

/**
* Query whether any duplicated BibTeX keys have been found in the database.
*
* @return true if there is at least one duplicate key.
*/
public boolean hasDuplicateKeys() {
return !duplicateKeys.isEmpty();
}

/**
* Get all duplicated keys found in the database.
*
* @return A list containing the duplicated keys.
*/
public List<String> getDuplicateKeys() {
return duplicateKeys;
}

public boolean isInvalid() {
return invalid;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.jabref.model.cleanup.FieldFormatterCleanup;
import org.jabref.model.cleanup.FieldFormatterCleanups;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.InternalField;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.util.DummyFileUpdateMonitor;

Expand Down Expand Up @@ -66,7 +65,7 @@ public void doPostCleanup(BibEntry entry) {

FieldFormatterCleanups cleanups = new FieldFormatterCleanups(true,
List.of(
new FieldFormatterCleanup(InternalField.TIMESTAMP, new ClearFormatter()),
new FieldFormatterCleanup(StandardField.TIMESTAMP, new ClearFormatter()),
// unescape the the contents of the URL field, e.g., some\_url\_part becomes some_url_part
new FieldFormatterCleanup(StandardField.URL, new LayoutFormatterBasedFormatter(new RemoveLatexCommandsFormatter()))
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.FieldProperty;
import org.jabref.model.entry.field.InternalField;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.types.EntryTypeFactory;
import org.jabref.model.metadata.MetaData;
Expand Down Expand Up @@ -141,11 +140,7 @@ public ParserResult parse(Reader in) throws IOException {

skipWhitespace();

try {
return parseFileContent();
} catch (KeyCollisionException kce) {
throw new IOException("Duplicate ID in bibtex file: " + kce);
}
return parseFileContent();
}

private void initializeParserResult() {
Expand Down Expand Up @@ -246,12 +241,7 @@ private void parseAndAddEntry(String type) {
// store complete parsed serialization (comments, type definition + type contents)
entry.setParsedSerialization(commentsAndEntryTypeDefinition + dumpTextReadSoFarToString());

boolean duplicateKey = database.insertEntry(entry);
if (duplicateKey) {
entry.getField(InternalField.KEY_FIELD).ifPresent(
key -> parserResult.addDuplicateKey(key)
);
}
database.insertEntry(entry);
} catch (IOException ex) {
// Trying to make the parser more robust.
// If an exception is thrown when parsing an entry, drop the entry and try to resume parsing.
Expand All @@ -278,7 +268,7 @@ private void parseJabRefComment(Map<String, String> meta) {
String comment = buffer.toString().replaceAll("[\\x0d\\x0a]", "");
if (comment.substring(0, Math.min(comment.length(), MetaData.META_FLAG.length())).equals(MetaData.META_FLAG)) {

if (comment.substring(0, MetaData.META_FLAG.length()).equals(MetaData.META_FLAG)) {
if (comment.startsWith(MetaData.META_FLAG)) {
String rest = comment.substring(MetaData.META_FLAG.length());

int pos = rest.indexOf(':');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
import org.jabref.model.entry.Month;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.InternalField;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.field.UnknownField;
import org.jabref.model.entry.types.StandardEntryType;
Expand Down Expand Up @@ -372,7 +371,7 @@ private void parseArticle(PubmedArticle article, List<BibEntry> bibItems) {
}

fields.put(StandardField.PMID, medlineCitation.getPMID().getContent());
fields.put(InternalField.OWNER, medlineCitation.getOwner());
fields.put(StandardField.OWNER, medlineCitation.getOwner());

addArticleInformation(fields, medlineCitation.getArticle().getContent());

Expand Down
Loading

0 comments on commit c7d7767

Please sign in to comment.