If the overhead isn't simply in the "get metadata" step, but in the "parsing metadata" one, you might want to restrict the search to very specific fields (e.g., title/author/series). I can certainly see the appeal of including description in there, but that might very well be your bottleneck right there... ;o).

Sidebar: This is nearly instantaneous if done via the calibre plugin/metadata ;p.

Description in fb2 books from Flibusta contains important information like translators, publushers etc.
Maybe make separate checkboxes to search in title, authors, series, description?

Maybe make separate checkboxes to search in title, authors, series, description?

If it actually helps the performance and doesn't look too messy, yeah?

I don't think that 1 or 4 metadata fields lookup would change anything (and neither html2plaintext) - the bottleneck is opening each book.
So, I'd rather keep that simple and a single checkbox (given the non neglieable amount of EPUBs I have were the title and author are swapped in their metadata).

Isn't that info in the SQLite DB?

If the coverbrowser plugin is enabled, it may be, for directories browsed - and it may not for sub/directories not visited or not "Extract and cache book information", or recently added books.
It's also in the docsettings of already opened books.

About the coverbrowser sqlite db, I'd like to mention that if we were to need a proper sqlite database for the purpose of metadata search, or a "library view" (ala Nickel, that some users would prefer to our file browser), I would prefer it to be another sqlite database. It could be a new plugin that could duplicate and extend bookinfomanager.lua if needs be.
Because the coverbrowser db is easy to handle: it's just a cache, we can trash it or entries, forget about deleted or moved books, replace lines when needing thumbnails of a different size - I wouldn't want it to get more complicated because of other reasons.
Another plugin with real care for complete metadata would need to keep track of deletions/move/addition and do regular scans for new books. Moreover, I think it would need to propose metadata edition to users, so they can fix/rewrite crap ones. Then, that metadata becomes more precious, and a lot more care (and more complex SQL) is needed to manage it.

(Or such a DB should be a "core" service, that coverbrowser could be a client to - but then, KOReader would no longer be the KOReader I like :)

For me the metadata is fine as is. :)

Search results view is now consistent with the file browser view (sorting, filtering, classic display mode).
To avoid code duplication FileChooser has been refactored and some of its methods are used in FileSearch.

FileChooser change: in classic mode files mandatory now shows the parameter by which sorting is done.

Minor wordings changes of the file searcher dialog window:

On tap in search results:

Fine with the switch of tap/long-press in the results.
For the code changes, too much stuff I'm not familiar with, so trusting you :)

Not a benchmark but rough estimation (a folder with 5 epubs of about 2 MB each): searching in metadata of an unopened book is ~50 times slower comparing to an opened one (with sdr).
Opening a book (even to get props only) is definitely the bottleneck.

Various thoughts:

You could add a small wrapper around BookInfoManager:getBookInfo(filepath) in coverbrowser's main.lua, so you could use self.ui.coverbrowser:getBookInfo(filepath) if self.ui.coverbrowser is available. You may get stalled info if the book metadata has been edited later, but so you would when reading from docsettings if the book has not been reopened since.
You could count the number of "miss" where you needed to open the book, and show a warning "You can speed things up by using Extract and cache book information".
(This would do a sql query for each book, so 1000 select if 1000 books - instead of using a single sql with title like '%metadata%' and authors like %metadata%', but that's bearable and still a lot speeder than opening each book.)

You could also force this extraction for each book - although that might be complicated and ugly (it's done in a background subprocess asynchronously, and it usually fetches the cover with the expected size for the current coverbrowser view).

Or you could even not rely on coverbrowser, and just cache (dump()/load()) a big table with all the metadata you have extracted once - so it is "slow" only the first time it is used, and when new files are met.

Also, I haven't look at all the code, but I guess this is blocking: if you have 300 never opened book, KOReader will go at opening the 300 of them, and the user can't stop that - and he may get stuck for 15mn...
Solutions for that are either using Trapper, or calling the search function with runInSubprocess(), so it can be interrupted by the main process. Both solutions are complicated though :/

Or you could even not rely on coverbrowser, and just cache (dump()/load()) a big table with all the metadata you have extracted once - so it is "slow" only the first time it is used, and when new files are met.

And if you would do that, it could be a core module, just caching filepath/filetimestamp/{metadata}, that you could just complete and iterate. May be you could even use LuaData to add new item to it, like we do for dict lookup queries.
We would not even need sqlite stuff: it's not that we could have 1 millions of books - loading 5000 books with each 1000 bytes of metadata should be fast.
(For coverbrowser, I needed sqlite mostly to allow communication between the main process (reading the DB to see if the book is done being indexed) and the subprocess (inserting into the DB), and sqlite itself avoiding concurrency with locks and ensuring all works well.)

Thank you.
I believe the cache is needed, this is not good to have such a slow search on every search.
Should we implement the metadata search without the cache and then discuss and enhance the cache realization in a new PR?

(At a first glance I tend to extending the DocSettings module to maintain the new DocPropsCache that would be a new file with hashes [filepath] = {book_props}. It would be updated within DocSettings:update(), and on every search, and by coverbrowser. And manually on a "Scan library" menu item. Needs more thinking and discussing).

Should we implement the metadata search without the cache and then discuss and enhance the cache realization in a new PR?

Dunno, the current state could block the user for 15 minutes with no way to get out but by powering off, so it feels like a non-feature :/ (unless he looks in a small subtree, or is used to opening all his books).

extending the DocSettings module to maintain the new DocPropsCache that would be a new file with hashes [filepath] = {book_props}.

Feels like the simpler solution, a "cache in the middle" :)
But it also feels limited and redundant with what CoverBrowser has, different code (sqlite vs Lua table) achieving the same purpose (caching metadata) with the same caveats (no editing, no handling of removed/moved books...)

It would be updated within DocSettings:update()

Well, I wouldn't want that big cache file to be rewritten each time I switch books, so you'd have to look if metadata has changed to see if it really need updates.

Needs more thinking and discussing

Yep, and I was happy not having to think about it until you came up with this PR :/ :)
I think if you are going to do that, you should think broader, and keep in mind that it should be reusable by a "Library view" plugin (#8472) and provide anything this could need.
It feels like it then better be a sqlite db. A clone of bookinfomanager.lua (ie. copy that, let CoverBrowser alone in the whole meantime, keeping in mind it should also be usable by coverbrowser, and coverbrowser should switch to using it in the end).
Some thoughs I had yesterday around that idea:

• the data for a book should have the timestamp of the file, so we can notice the book file mod time has been updated and the cached metadata can be stalled
• it should have a edited=true/false, and allow the user to edit title/authors, with new text either replacing the one extracted for the book, or may be better: stored in additiona fields
• the cover bb should may be stored in another SQL table, with versions for the multiple size (ie. small detailed list mode vs mosaic mode - currently, I replace small ones by larger ones, so some downsizing may happen when you browse detailed list)

Anyway, lots of thinking to give it - and not really happy about having to give it that much :)

I agree with the above. I'm not motivated to create something like a library view, but if you're implementing something along these lines I don't think it's a very good idea unless you take CoverBrowser as a point of departure and add some well considered extra columns.

Should we implement the metadata search without the cache and then discuss and enhance the cache realization in a new PR?

Dunno, the current state could block the user for 15 minutes with no way to get out but by powering off, so it feels like a non-feature :/ (unless he looks in a small subtree, or is used to opening all his books).

But if you want to keep that (and may be this will be enough, and we can postpone that dreadful thinking :), you could:

• implement the coverbrowser:getBookInfo(filepath) I mentionned, and use only that (no docsetting opening needed unless you really want to, and surely no book opening)
• disable this metadata checkbox if coverbrowser not enabled
• if you got any miss, just report it to the user, and suggest "Extract and cache..."

Possibly because of resetState() (losing the hold state) done by inhibitInput/restoreInput when opening a book - and now when show folder.

Yeah, that sounds sensible. You can't call resetState during a "complex" gesture (i.e., anything that isn't a tap or a key), as it'll break it down, resulting in a tap on contact lift.

(I'll look at the logs tonight to confirm, but that's probably it).

EDIT: Yup, definitely that ;).

I've made a simple benchmark to compare the speed of getting book props from sdr vs coverbrowser sql.
For previously opened books DocSettings:open() is about 9 times slower than coverbrowser:getBookInfo().
I think it makes sense to get info from sql in classic mode too.
And add the "Extract book info..." button to the classic mode Plus menu.

You mean in the specific scenario of search while in classic mode? Yes, I think it would make sense to say that completely classic search for metadata doesn't make much sense.

Btw, where'd your run the benchmark? If that was on the emulator then DocSettings:open() may have performed significantly better than on a real device due to I/O bottlenecks.

I ran the benchmark on my Kindle Voyage.

I mean using the coverbrowser sql base in the file popup menu in FM, History and Favorites:

I mean using the coverbrowser sql base in the file popup menu in FM, History and Favorites:

I suppose? In that case the difference seems immaterial; it feels instantaneous either way. Are you talking about when there's no metadata.lua yet? It seems hard to imagine it'd be 9 times slower?

Yes, the difference is not visible per one file.
Only we would know that we do it faster, with less power consumption, and without code duplication (fudging buttons in CoverBrowser).

What does your benchmark consist of exactly? I find it hard to imagine that simply parsing a tiny bit of Lua is not only heavier than instantiating all of SQLite but an order of magnitude so. But I have heard that the filesystem on Kindle is unbelievably slow…

It was a previously opened book with metadata extracted to sql.

logger.info("doc_settings start", os.clock())
for i = 1, 1000 do
    local doc_settings = DocSettings:open(item.path)
    local descr = doc_settings:readSetting("doc_props").description
end
logger.info("doc_settings end", os.clock())
logger.info("bookinfo start", os.clock())
for i = 1, 1000 do
    local bookinfo = self._manager.ui.coverbrowser:getBookInfo(item.path)
    local descr = bookinfo.description
end
logger.info("bookinfo end", os.clock())

os.clock may be misleading (it only counts CPU time of the Lua process), which may be very far from the actual real world time ;).

The saner way to do this is via time, e.g.,

I don't see such a drastic difference, closer to twice as slow. According to the profiler up to about 25% of the difference comes from getHistoryPath, basename and buildCandidates, but without doing anything about that the benchmark comes to the same result if you limit the metadata to basically just metadata, along these lines (see #2789):

-- we can read Lua syntax here!
return {
    ["doc_pages"] = 250,
    ["doc_path"] = "/home/frans/src/kobo/koreader/test/juliet.epub",
    ["doc_props"] = {
        ["authors"] = "William Shakespeare",
        ["description"] = "Romeo and Juliet is a tragic play written early in the career of William Shakespeare about two teenage \"star-cross'd lovers\" whose untimely deaths ultimately unite their feuding households. It was among Shakespeare's most popular plays during his lifetime and, along with Hamlet, is one of his most frequently performed plays. Today, the title characters are regarded as archetypal \"young lovers\". (From Wikipedia)",
        ["keywords"] = "Fiction\
Drama\
Romance",
        ["language"] = "en",
        ["series"] = "",
        ["title"] = "Romeo and Juliet",
    },
}

But I don't know why you'd make things complicated for something that isn't slow and that you only do irregularly.

PS For fun you can throw a closeDbConnection() in there and watch the time increase by an order of magnitude over the plain file method. I suspect that that's more realistic, at least for my usage. Unless you keep it open even when switching between reader and filemanager.

So, the summary of changes in this PR:

(1) Search files
The dialog includes a checkbox to search in book metadata (default unchecked).
Metadata is taken from the coverbrowser sql base, or from the book sdr file. No book opening is done. The books without metadata in sql/sdr are skipped in search and the dialog proposing to extract the metadata is shown.
Technically: the search results list now is based on the FileChooser code, so all the sorting and filtering parameters are applicable to the list.

(2) File sorting
A new separate checkbox for mix sorting of folders and files, available in all sorting modes except "type", "size", "percent finished", where folders are always sorted by name and shown on top of the list.
In classic mode the mandatory of the file list shows the value of the sorting parameter. So we understand the files order. For sorting by name the file size is shown, as usual.

Works good I hope.

The changes to FileChooser in this PR have broken the unreadable dir content handling (like going up from Android /storage/emulated/0).
The piece of code that handles the "unreadable dir content" was not changed (pcall and ok check).
I cannot reproduce the bug on Kindle, there are no unreadable partitions here.

I can simulate that on the linux emulator with chmod 111 some upper parent directory.
The crash I get when going up is:

./luajit: frontend/ui/widget/filechooser.lua:271: attempt to concatenate field 'text' (a nil value)
stack traceback:
 frontend/ui/widget/filechooser.lua:271: in function 'genItemTableFromPath'
 frontend/ui/widget/filechooser.lua:391: in function 'refreshPath'
 frontend/ui/widget/filechooser.lua:415: in function 'changeToPath'
 frontend/ui/widget/filechooser.lua:474: in function 'onMenuSelect'

Hope this helps :)