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 of journal abbreviation loading #8928

Closed
Siedlerchr opened this issue Jun 24, 2022 · 2 comments · Fixed by #9515
Closed

Improve performance of journal abbreviation loading #8928

Siedlerchr opened this issue Jun 24, 2022 · 2 comments · Fixed by #9515

Comments

@Siedlerchr
Copy link
Member

Siedlerchr commented Jun 24, 2022

Is your suggestion for improvement related to a problem? Please describe.
As a User I have a huge list of custom abbreviations. Adding them will freeze JabRef for a couple of minutes and will produce other issues.
https://discourse.jabref.org/t/journal-abbreviations-with-dot/3400/4

Describe the solution you'd like
I would like to have a better performance and continue using JabRef while the abbreviations are loaded in the background.

Additional context
JabRef stores its internal abbreviations in a h2 mv database. Custom abbreviations are added to it. Refs #6304
The key problem is that all journal abbreviation stuff loading is done on the FX-Thread. This will block the user.

Solution: The files need to be loaded in a background task and added to JabRef's mv database.
It must be taken care of concurrent reading/writing issues when toggling the abbreviation button in the entry editor.
It must be considered that the journal Repo is injected everywhere.

@HoussemNasri
Copy link
Member

HoussemNasri commented Jun 24, 2022

Apparently, the real bottleneck reside in the way duplicates are removed. Before an abbreviation entry gets added to the list, it will be searched and removed to make sure no duplicates are kept. I run the profiler and found 99.5% of abbreviation loading time goes to ArrayList#remove. This clearly happened because Lists are not optimized for removal. One possible solution for the bottleneck is to replace the list with another data structure, maybe a Set or a LinkedList.
Nonetheless the whole thing should not run on the FX Thread. The FX Thread should never do long running tasks. This will block the UI.

Bottleneck

public void addCustomAbbreviation(Abbreviation abbreviation) {
Objects.requireNonNull(abbreviation);
// We do not want to keep duplicates, thus remove the old abbreviation
// (abbreviation equality is tested on name only, so we cannot use a Set instead)
customAbbreviations.remove(abbreviation);
customAbbreviations.add(abbreviation);
}

Profiler Results

image

@calixtus calixtus added this to the v5.8 milestone Aug 15, 2022
@koppor koppor modified the milestones: v5.8, v6.0 Sep 26, 2022
@koppor koppor moved this to Normal priority in Features & Enhancements Nov 7, 2022
@ThiloteE
Copy link
Member

ThiloteE commented Jan 2, 2023

Metaissue: #8906

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants