search

Kunzisoft / KeePassDX

Lightweight vault and password manager for Android, KeePassDX allows editing encrypted data in a single file in KeePass format and fill in the forms in a secure way.
https://www.keepassdx.com/
GNU General Public License v3.0
4.32k stars 261 forks source link

Search got slower #964

Closed zinalili closed 3 years ago

zinalili commented 3 years ago

Now I'm on 2.9.18 and on earlier versions Search was faster. Is there an option to revert it to how it was before?

J-Jamet commented 3 years ago

The search conditions have been changed to satisfy users, but this should affect overall performance if you have a lot of data. Linked to https://github.com/Kunzisoft/KeePassDX/issues/962 https://github.com/Kunzisoft/KeePassDX/issues/945

I can make parameters to activate/deactivate these new conditions, it should speed up the process.

J-Jamet commented 3 years ago

KeePassDX-2.9.19_Test_Search_A.zip

Can you tell me if it's better with this build? I improved the method that replaces the accents, it should be better but I don't know if this improvement is enough for you? (Because if I can avoid making new settings now, I'd be happy. :D)

zinalili commented 3 years ago

It is better now but only a little. Not exactly the same how it was before. Please be happy, take your time and make new settings whenever you feel like doing it :-D

MichiFr commented 3 years ago

I've got many entries in my v4 database and searching completely freezes the app. No matter if I use the beta version or the final from store.

Maybe it's not a freeze but I did not wait for the app to finish because the result should be almost instant. Other Keepass aps show the search results within a second.

J-Jamet commented 3 years ago

Yes, I understand. The search code didn't include input references and ignoring accents, but with these new feature, as the search is based on the historical fork code there is a lack of optimization in this area. I am trying to improve it and I will implement the search features as disabled by default. So the speed should be the same as before (even better if I optimize correctly), but I have to finish the code. My tests passed without seeing this drop in performance in the search, sorry about that. I'm doing my best to solve the problem.

MichiFr commented 3 years ago

Are those changes related to how the app determines which suggestions it shows within a web page?

I'm asking because on desktop Keepass shows a lot of more entries that could fit for the current page and KeepassDX does only show a small excerpt of them. Sometimes the correct entry is even missing and obviously I cannot force KeepassDX to let me select a different one from within this little popup.

Maybe the algorithm is diferent from that which the Firefox plugin uses to find suitable credentials in my password file.

J-Jamet commented 3 years ago

Are those changes related to how the app determines which suggestions it shows within a web page?

Yes, it uses these methods internally. The search is made according to the applicationId or the webDomain of the application / browser that proposes the directory. The proposed entries must contain one of these two components. https://github.com/Kunzisoft/KeePassDX/wiki/AutoFill#auto-search

J-Jamet commented 3 years ago

KeePassDX-3.9.19_Test_Search.zip

OK, I've made a lot of improvements but I'd like to see your feedback before I roll out the update. Can you tell me if all 3 APKs work well or only some or none for you? If you have slowdowns or not at all. Thanks. :) You have to delete the application between each APK installation.

MichiFr commented 3 years ago

Thanks for this really quick test versions! I just started with version D and can confirm: much faster than before. To be precisely, the search as you type function works instantly. Great job!

I've discovered two issues though,which may be already reported and not related in any way with those test version. I will file a new ticket if not already done.

Thanks again!

J-Jamet commented 3 years ago

OK thank you for the feedback.

If test D works, that's great, but does test B work? because : A - Old code, better flatten to ASCII char conversion, search in field reference B - New code, char conversion, field reference C - New code, no char conversion, field reference D - New code, char conversion, no field reference

Ideally I would like to have both features in addition to avoid regression in functionality #962 (field reference) and #945(char conversion) but I can only deploy if you tell me that there is no problem with B and C.

MichiFr commented 3 years ago

Version B works, too, however, much lower than D. Version C works as well, similar to B, maybe slightly slower. In both cases, when start typing in the searchbox, the matching entries are dropping in one after another. Not very fluently but at least no freeze.

And of course field references should still be resolved, I still need it 😁

J-Jamet commented 3 years ago

OK, Thank you. So, I'll have to improve the algorithm again but I can't do it forever. So for the moment I remove the field references and make them a parameter when I'll work on it later.

J-Jamet commented 3 years ago

KeePassDX-3.9.19_Test_SearchE.zip

OK, I've redone the field reference engine which doesn't make useless loops anymore and instead of parsing the tree of entries, I search directly in the database map from the UUID, it should be faster. But I don't know if it's still fast enough for you, so let me know if you are satisfied with this E version.

MichiFr commented 3 years ago

Thanks for the update, I've just installed it and search works as expected, not super fast but normal 😊

OTH, field references are not resolved any more. Which mean when opening an account with refer3nc3s in password or username, I see the UUID and for example pressing the little copy button on the right side does just copy the content, the reference string, but not the password or username itself.

This applies to any login web page, too.

J-Jamet commented 3 years ago

OTH, field references are not resolved any more. Which mean when opening an account with refer3nc3s in password or username, I see the UUID and for example pressing the little copy button on the right side does just copy the content, the reference string, but not the password or username itself.

OK, this is not normal, it may be due to the new verification methods, I'll look at where the problem comes from.

J-Jamet commented 3 years ago

KeePassDX-3.9.19_Test_SearchF.zip

And another one. Sorry for all these packages, here I tested and retested and optimized again, it will be difficult to do better for the moment.

MichiFr commented 3 years ago

Good job! Field references are now working in account details and when autofilling web pages.

One small issue though: I've activated the option to show the username in any lists on the second line. When searching for any account, that contains a reference in username, the preview shows the UUID and reference string instead of the resolved content.

Nevertheless I can search for any content the field reference points to. E.g. the referenced username contains S012345, the search does find all accounts with this name even those which contain a reference.

J-Jamet commented 3 years ago

~I can't reproduce the bug you describe. When the setting is activated, the decoded reference is displayed in the second line. What type of reference is it? If you could upload a test database with the problem it would be great.~

J-Jamet commented 3 years ago

Ho! I just figured it out, it's in the search items and not in the basic list. I'm solving the problem and deploying the app, I hadn't seen that. My bad.