-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Improve performance #5539
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 #5539
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ | |
| import org.jabref.model.entry.types.StandardEntryType; | ||
| import org.jabref.model.strings.LatexToUnicodeAdapter; | ||
| import org.jabref.model.strings.StringUtil; | ||
| import org.jabref.model.util.MultiKeyMap; | ||
|
|
||
| import com.google.common.base.Strings; | ||
| import com.google.common.eventbus.EventBus; | ||
|
|
@@ -53,14 +54,22 @@ public class BibEntry implements Cloneable { | |
| private static final Logger LOGGER = LoggerFactory.getLogger(BibEntry.class); | ||
| private static final Pattern REMOVE_TRAILING_WHITESPACE = Pattern.compile("\\s+$"); | ||
| private final SharedBibEntryData sharedBibEntryData; | ||
|
|
||
| /** | ||
| * Map to store the words in every field | ||
| */ | ||
| private final Map<Field, Set<String>> fieldsAsWords = new HashMap<>(); | ||
|
|
||
| /** | ||
| * Cache that stores latex free versions of fields. | ||
| */ | ||
| private final Map<Field, String> latexFreeFields = new ConcurrentHashMap<>(); | ||
|
|
||
| /** | ||
tobiasdiez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * Cache that stores the field as keyword lists (format <Field, Separator, Keyword list>) | ||
| */ | ||
| private MultiKeyMap<Field, Character, KeywordList> fieldsAsKeywords = new MultiKeyMap<>(); | ||
|
|
||
| private final EventBus eventBus = new EventBus(); | ||
| private String id; | ||
| private final ObjectProperty<EntryType> type = new SimpleObjectProperty<>(DEFAULT_TYPE); | ||
|
|
@@ -720,8 +729,7 @@ public void addKeywords(Collection<String> keywords, Character delimiter) { | |
| } | ||
|
|
||
| public KeywordList getKeywords(Character delimiter) { | ||
| Optional<String> keywordsContent = getField(StandardField.KEYWORDS); | ||
| return keywordsContent.map(content -> KeywordList.parse(content, delimiter)).orElse(new KeywordList()); | ||
| return getFieldAsKeywords(StandardField.KEYWORDS, delimiter); | ||
| } | ||
|
|
||
| public KeywordList getResolvedKeywords(Character delimiter, BibDatabase database) { | ||
|
|
@@ -824,33 +832,46 @@ public Set<String> getFieldAsWords(Field field) { | |
| } | ||
| } | ||
|
|
||
| public KeywordList getFieldAsKeywords(Field field, Character keywordSeparator) { | ||
| Optional<KeywordList> storedList = fieldsAsKeywords.get(field, keywordSeparator); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't you use Google Guava's Cache? 😇 Maybe, with Eclipse Collections Twin LoadingCache<Pair<Field, Character>, KeywordList> storedKeywordList = CacheBuilder.newBuilder()
.build(new CacheLoader<Pair<Field, Character>, KeywordList>() {
@Override
public String load(final Pair<Field, Character> pair) throws Exception {
return getField(pair.getOne().map(... pair.getTwo());
}
});return storedKeywordList.get(Tuples.pair(field, keywordSeparator));OK, invalidation is worse (as it is not possible to invalidate all pairs having the same first value (easily)). storedKeywordList.invalidateAll();(I now that Tuples.pair(...) might be slower than your approach. Howver, I would like that the cache can be tweaked ^^).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That looks complicated 😄 https://github.com/google/guava/wiki/CachesExplained#applicability has the nice explanation
I guess we do not need these cache features (but I've also never worked with Guave Caches...so I cannot appreciate them). |
||
| if (storedList.isPresent()) { | ||
| return storedList.get(); | ||
| } else { | ||
| KeywordList keywords = getField(field) | ||
| .map(content -> KeywordList.parse(content, keywordSeparator)) | ||
| .orElse(new KeywordList()); | ||
| fieldsAsKeywords.put(field, keywordSeparator, keywords); | ||
| return keywords; | ||
| } | ||
| } | ||
|
|
||
| public Optional<FieldChange> clearCiteKey() { | ||
| return clearField(InternalField.KEY_FIELD); | ||
| } | ||
|
|
||
| private void invalidateFieldCache(Field field) { | ||
| latexFreeFields.remove(field); | ||
| fieldsAsWords.remove(field); | ||
| fieldsAsKeywords.remove(field); | ||
| } | ||
|
|
||
| public Optional<String> getLatexFreeField(Field field) { | ||
| if (!hasField(field) && !InternalField.TYPE_HEADER.equals(field)) { | ||
| return Optional.empty(); | ||
| } else if (latexFreeFields.containsKey(field)) { | ||
| return Optional.ofNullable(latexFreeFields.get(field)); | ||
| } else if (InternalField.KEY_FIELD.equals(field)) { | ||
| if (InternalField.KEY_FIELD.equals(field)) { | ||
| // the key field should not be converted | ||
| Optional<String> citeKey = getCiteKeyOptional(); | ||
| latexFreeFields.put(field, citeKey.get()); | ||
| return citeKey; | ||
| return getCiteKeyOptional(); | ||
| } else if (InternalField.TYPE_HEADER.equals(field)) { | ||
| String typeName = type.get().getDisplayName(); | ||
| latexFreeFields.put(field, typeName); | ||
| return Optional.of(typeName); | ||
| return Optional.of(type.get().getDisplayName()); | ||
| } else if (latexFreeFields.containsKey(field)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could also be a good place for Guava's Cache. |
||
| return Optional.ofNullable(latexFreeFields.get(field)); | ||
| } else { | ||
| String latexFreeField = LatexToUnicodeAdapter.format(getField(field).get()).intern(); | ||
| latexFreeFields.put(field, latexFreeField); | ||
| return Optional.of(latexFreeField); | ||
| Optional<String> fieldValue = getField(field); | ||
| if (fieldValue.isPresent()) { | ||
| String latexFreeField = LatexToUnicodeAdapter.format(fieldValue.get()).intern(); | ||
| latexFreeFields.put(field, latexFreeField); | ||
| return Optional.of(latexFreeField); | ||
| } else { | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package org.jabref.model.util; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| public class MultiKeyMap<K1, K2, V> { | ||
tobiasdiez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| private final Map<K1, Map<K2, V>> map = new HashMap<>(); | ||
|
|
||
| public Optional<V> get(K1 key1, K2 key2) { | ||
| Map<K2, V> metaValue = map.get(key1); | ||
| if (metaValue == null) { | ||
| return Optional.empty(); | ||
| } else { | ||
| return Optional.ofNullable(metaValue.get(key2)); | ||
| } | ||
| } | ||
|
|
||
| public void put(K1 key1, K2 key2, V value) { | ||
| Map<K2, V> metaValue = map.get(key1); | ||
| if (metaValue == null) { | ||
| Map<K2, V> newMetaValue = new HashMap<>(); | ||
| newMetaValue.put(key2, value); | ||
| map.put(key1, newMetaValue); | ||
| } else { | ||
| metaValue.put(key2, value); | ||
| } | ||
| } | ||
|
|
||
| public void remove(K1 key1) { | ||
| map.remove(key1); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.