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

Fast search #1100

Merged
merged 13 commits into from Apr 6, 2016
Merged

Fast search #1100

merged 13 commits into from Apr 6, 2016

Conversation

@simonharrer
Copy link
Contributor

simonharrer commented Apr 4, 2016

  • Fix performance bug which caused the UI to slow down extremely

Search is now blazingly fast (compared to before)

  • New class MainTableDataModel which holds all sorted and filtered lists plus sorting and filtering logic.
  • If a search is still active while entering a new search term, the previous search is cancelled. This improves search time quite a lot.
  • By filtering after the sorting, the search is extremely fast as the amount of sorting is reduced dramatically
  • Using "filter" search should be now the fastest search, as no sorting has to take place

Issues

  • bugs using the float search (but filter works as expected)
  • remove sysout statement
  • tests fail as the mode of the database is written back directly to the file

Please try this out.

Related

  • #1094 as this improves performance of the main table (but without numbers)
  • #1034 may no longer be required to be fixed in #1037
simonharrer added 5 commits Apr 4, 2016
- New class MainTableDataModel which holds all sorted and filtered lists plus sorting and filtering logic.
- If a search is still active while entering a new search term, the previous search is cancelled. This improves search time quite a lot.
- By filtering after the sorting, the search is extremely fast as the amount of sorting is reduced dramatically
- Using "filter" search should be now the fastest search, as no sorting has to take place
…ry quickly (5 sec vs. 60 sec for 40k entries)
@stefan-kolb stefan-kolb added this to the v3.3 milestone Apr 5, 2016
@stefan-kolb
Copy link
Member

stefan-kolb commented Apr 5, 2016

L(fast)TM 👍

@tobiasdiez tobiasdiez mentioned this pull request Apr 5, 2016
0 of 3 tasks complete
@tobiasdiez
Copy link
Member

tobiasdiez commented Apr 5, 2016

LGTM 👍

simonharrer added 3 commits Apr 5, 2016
@simonharrer simonharrer force-pushed the fast-search branch from 5bf418e to e0380b7 Apr 5, 2016
@tobiasdiez

This comment has been minimized.

I really don't like this...I suspect moving YearUtil to logic is also not possible, right?

This comment has been minimized.

Copy link
Contributor Author

simonharrer replied Apr 5, 2016

Nope, not possible. But I see no other solution except not to make this improvement.

This comment has been minimized.

Copy link
Member

tobiasdiez replied Apr 5, 2016

Remove the distinction between model and logic? But I'm probably the only one which does not likes the model package...

This comment has been minimized.

Copy link
Contributor Author

simonharrer replied Apr 5, 2016

Even if we would combine model and logic, we would have very bad dependency relations. By separating model and logic, they are simply revealed there.

@Siedlerchr

This comment has been minimized.

Copy link
Contributor

Siedlerchr commented on e0380b7 Apr 5, 2016

I really don't like the YearUtil class at all and I think it is obsolete, or at least its self created logic is obsolete.
As with the new Java 8 Release, there haven been important changes made to the Date-Logic.
Especially the new DateFormatters. Why don't use them instead of manually trying to do Date Arithmetic`?
http://www.tutorialspoint.com/java8/java8_datetime_api.htm
And here:
https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html#ofPattern-java.lang.String-

This comment has been minimized.

Copy link
Contributor Author

simonharrer replied Apr 5, 2016

I think this Year arithmetic cannot be easily handled with Java 8 classes, but you are welcome to try it.

This comment has been minimized.

Copy link
Contributor

Siedlerchr replied Apr 5, 2016

I will look into that.

@simonharrer simonharrer merged commit 756be32 into master Apr 6, 2016
3 of 6 checks passed
3 of 6 checks passed
codacy/pr Not so good... This commit quality could be better.
Details
codecov/project 23.60% (-0.01%) compared to a97c720 at 23.61%
Details
coverage/coveralls Coverage decreased (-0.002%) to 27.276%
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@simonharrer simonharrer deleted the fast-search branch Apr 6, 2016
@tobiasdiez
Copy link
Member

tobiasdiez commented Apr 6, 2016

Running the benchmarks with this PR merged in shows that the writing speed was improved by an additional factor of 10 (being now at roughly 60k entries per second). Good job 👍

@mlep
Copy link
Contributor

mlep commented Apr 6, 2016

Considering users reported JabRef to be slow, this is a great improvement (worth being advertised in the blog!).

@simonharrer
Copy link
Contributor Author

simonharrer commented Apr 6, 2016

The problem is, I think currently we lack people writing these blog posts... :)

@mlep
Copy link
Contributor

mlep commented Apr 6, 2016

@simonharrer: A problem? Here is the stub of a solution.
I collated various information from different issues. I am not sure I got everything right, hence, @JabRef/developers, feel free to improve it!
Note: I think we should cite the names of people doing the work (code does not come out of the blue, right?), and invite users to give us feedback.

For later on: who is in charge of the blog?

%%%%%%%%%%%%%%%%%%%%%%%%%%

A faster JabRef is coming!

Some users reported JabRef was slow on large databases (thank you for the feedback!). This was especially the case for three operations:

  • loading a database
  • saving a database
  • searching through a database.

During a search, the user interface could become very unresponsive, which is indeed quite annoying... Well, this time will be over soon: Programmer @simonharrer has recently contributed code making search much faster (#1100).

Preliminary tests have been carried out on a database with 100000 entries (is it big enough for you?). They shows that JabRef is now 10 times faster at searching: about 60000 entries are searched per second.

If you want to give a try: http://builds.jabref.org/master/ (this is a development version. So, be carefully, and back up your data!). Your comments are welcomed at #1100

About opening large databases: work is under way to speed it up You can follow this development at #1094

@stefan-kolb
Copy link
Member

stefan-kolb commented Apr 6, 2016

Great, now we need the blog of the Stupro project so the students and alsomlep and other can add small blog posts!

@simonharrer
Copy link
Contributor Author

simonharrer commented Apr 6, 2016

the blog is already online, I think.

@mlep
Copy link
Contributor

mlep commented Apr 6, 2016

It is here: http://www.jabref.org/blog/
For a new post, one has simply to add a new .md file with the proper header to https://github.com/JabRef/www.jabref.org/tree/gh-pages/_posts, right?

@lc9275
Copy link

lc9275 commented Apr 6, 2016

@mlep
I have a database of about 1300 articles. What is really slow with Jabref is scrolling (using either the wheel or dragging the scrollbar) - it is just never smooth.

@mlep
Copy link
Contributor

mlep commented Apr 6, 2016

@lc9275 I have a database the same as as yours, and scrolling is ok.
First of all, could you confirm scrolling is slow for you when using the last development version (on a copy of your database): http://builds.jabref.org/master/

@lc9275
Copy link

lc9275 commented Apr 6, 2016

@mlep That solved the problem, nice and smooth! I was using a dev version of a month or two ago. Thanks for the quick feedback, and I am happy it is solved in the current version.

@simonharrer
Copy link
Contributor Author

simonharrer commented Apr 7, 2016

@oscargus

This comment has been minimized.

@simonharrer Passing null here seems to be a bad idea since the matcher is directly dereferences in matches. Or am I (and Find bugs) missing something?

This comment has been minimized.

Copy link
Contributor

oscargus replied Jul 26, 2016

Same three line above.

@hankstevens01
Copy link

hankstevens01 commented Mar 20, 2020

Still slow responding to keyboard in Search. I type, and then have to wait several seconds for the characters to appear in the search dialogue. SEarch itself is fast. Typing in request lags.
Using Mac Catalina 10.15.3 w JabRef 5.1 downloaded 20 March.

@koppor
Copy link
Member

koppor commented Mar 23, 2020

@hankstevens01 Could you provide some insights on your concrete library you are working with? - I think, your issue is also described at #5071, isn't it?

@hankstevens01
Copy link

hankstevens01 commented Mar 24, 2020

@hankstevens01
Copy link

hankstevens01 commented Mar 24, 2020

@lc9275
Copy link

lc9275 commented Mar 24, 2020

I can confirm the search is still unworkable slow. My database only has 2000 entries. I think the search itself is the problem, as a search is performed each time a character is entered, instead of after the user presses return.

@koppor
Copy link
Member

koppor commented Mar 28, 2020

@lc9275 It was the wish of users to have search-as-you-type.

I put a comment at #5071 so that the search is not forgotten. We hope that we have enough example data bases to investigate the issue properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
You can’t perform that action at this time.