From 4d4b04359cb8dc9d17886e2b39d435aa8582d252 Mon Sep 17 00:00:00 2001 From: hius07 <62179190+hius07@users.noreply.github.com> Date: Tue, 25 Oct 2022 06:15:59 -0400 Subject: [PATCH] ReadHistory: refactoring with binary search (#9603) Get rid of indexing and sorting, reduce flushing. --- frontend/readhistory.lua | 283 ++++++++++++++++++--------------- spec/unit/readhistory_spec.lua | 23 --- 2 files changed, 151 insertions(+), 155 deletions(-) diff --git a/frontend/readhistory.lua b/frontend/readhistory.lua index ea2df2aa9..26a9d1d8a 100644 --- a/frontend/readhistory.lua +++ b/frontend/readhistory.lua @@ -22,14 +22,12 @@ end local function buildEntry(input_time, input_file) local file_path = realpath(input_file) or input_file -- keep orig file path of deleted files - local file_exists = lfs.attributes(file_path, "mode") == "file" return { time = input_time, - text = input_file:gsub(".*/", ""), file = file_path, - dim = not file_exists, -- "dim", as expected by Menu - -- mandatory = file_exists and util.getFriendlySize(lfs.attributes(input_file, "size") or 0), - mandatory_func = function() -- Show the last read time (rather than file size) + text = input_file:gsub(".*/", ""), + dim = lfs.attributes(file_path, "mode") ~= "file", -- "dim", as expected by Menu + mandatory_func = function() -- Show the last read time local readerui_instance = require("apps/reader/readerui"):_getRunningInstance() local currently_opened_file = readerui_instance and readerui_instance.document and readerui_instance.document.file local last_read_ts @@ -40,7 +38,7 @@ local function buildEntry(input_time, input_file) else -- For past documents, the last save time of the settings is better -- as last read time than input_time (its last opening time, that - -- we fallback to it no sidecar file) + -- we fallback to if no sidecar file) last_read_ts = DocSettings:getLastSaveTime(file_path) or input_time end return util.secondsToDate(last_read_ts, G_reader_settings:isTrue("twelve_hour_clock")) @@ -50,59 +48,61 @@ local function buildEntry(input_time, input_file) end, callback = function() selectCallback(input_file) - end + end, } end -local function fileFirstOrdering(l, r) - if l.file == r.file then - return l.time > r.time - else - return l.file < r.file +function ReadHistory:getIndexByFile(item_file) + for i, v in ipairs(self.hist) do + if item_file == v.file then + return i + end end end -local function timeFirstOrdering(l, r) - if l.time == r.time then - return l.file < r.file - else - return l.time > r.time +--- Returns leftmost index of the entry with item_time using binary search +-- (items in history are sorted by time in reverse order). +-- If several entries have equal time, search within them by item_file in alphabetical order. +-- If there are no entries with item_time, returns insertion index. +function ReadHistory:getIndexByTime(item_time, item_file) + local hist_nb = #self.hist + if hist_nb == 0 then + return 1 end -end - -function ReadHistory:_indexing(start) - --- @todo (Hzj_jie): Use binary search to find an item when deleting it. - for i = start, #self.hist, 1 do - self.hist[i].index = i + if item_time > self.hist[1].time then + return 1 + elseif item_time < self.hist[hist_nb].time then + return hist_nb + 1 end -end - -function ReadHistory:_sort() - local autoremove_deleted_items_from_history = - not G_reader_settings:nilOrFalse("autoremove_deleted_items_from_history") - if autoremove_deleted_items_from_history then - self:clearMissing() + local s, e, m, d = 1, hist_nb + while s <= e do + m = bit.rshift(s + e, 1) + if item_time < self.hist[m].time then + s, d = m + 1, 1 + else + e, d = m - 1, 0 + end end - table.sort(self.hist, fileFirstOrdering) - --- @todo (zijiehe): Use binary insert instead of a loop to deduplicate. - for i = #self.hist, 2, -1 do - if self.hist[i].file == self.hist[i - 1].file then - table.remove(self.hist, i) + local index = m + d + if item_file then + while index <= #self.hist + and self.hist[index].time == item_time + and self.hist[index].file:gsub(".*/", "") < item_file do + index = index + 1 end end - table.sort(self.hist, timeFirstOrdering) - self:_indexing(1) + return index end --- Reduces total count in hist list to a reasonable number by removing last --- several items. +--- Reduces number of history items to the required limit by removing old items. function ReadHistory:_reduce() - while #self.hist > 500 do - table.remove(self.hist, #self.hist) + local history_size = G_reader_settings:readSetting("history_size") or 500 + while #self.hist > history_size do + table.remove(self.hist) end end --- Flushes current history table into file. +--- Saves history table to a file. function ReadHistory:_flush() local content = {} for _, v in ipairs(self.hist) do @@ -123,23 +123,25 @@ end -- @treturn boolean true if the history_file has been updated and reloaded. function ReadHistory:_read(force_read) local history_file_modification_time = lfs.attributes(history_file, "modification") - if history_file_modification_time == nil - or (not force_read and (history_file_modification_time <= self.last_read_time)) then - return false + if history_file_modification_time == nil then -- no history_file, proceed legacy only + return true end - self.last_read_time = history_file_modification_time - local ok, data = pcall(dofile, history_file) - if ok and data then - self.hist = {} - for _, v in ipairs(data) do - table.insert(self.hist, buildEntry(v.time, v.file)) + if force_read or (history_file_modification_time > self.last_read_time) then + self.last_read_time = history_file_modification_time + local ok, data = pcall(dofile, history_file) + if ok and data then + self.hist = {} + for _, v in ipairs(data) do + table.insert(self.hist, buildEntry(v.time, v.file)) + end end + return true end - return true end --- Reads history from legacy history folder +--- Reads history from legacy history folder. function ReadHistory:_readLegacyHistory() + local history_updated local history_dir = DataStorage:getHistoryDir() for f in lfs.dir(history_dir) do local path = joinPath(history_dir, f) @@ -148,14 +150,20 @@ function ReadHistory:_readLegacyHistory() if path ~= nil and path ~= "" then local file = DocSettings:getNameFromHistory(f) if file ~= nil and file ~= "" then - table.insert( - self.hist, - buildEntry(lfs.attributes(joinPath(history_dir, f), "modification"), - joinPath(path, file))) + local item_path = joinPath(path, file) + local item_time = lfs.attributes(joinPath(history_dir, f), "modification") + if self:addItem(item_path, item_time, true) then + history_updated = true + end end end end end + if history_updated then + self:_reduce() + self:_flush() + self:ensureLastFile() + end end function ReadHistory:_init() @@ -163,10 +171,10 @@ function ReadHistory:_init() end function ReadHistory:ensureLastFile() - local last_existing_file = nil - for i=1, #self.hist do - if lfs.attributes(self.hist[i].file, "mode") == "file" then - last_existing_file = self.hist[i].file + local last_existing_file + for _, v in ipairs(self.hist) do + if lfs.attributes(v.file, "mode") == "file" then + last_existing_file = v.file break end end @@ -178,47 +186,63 @@ function ReadHistory:getLastFile() return G_reader_settings:readSetting("lastfile") end +--- Get last or previous file in history that is not current_file +-- (self.ui.document.file, provided as current_file, might have +-- been removed from history). function ReadHistory:getPreviousFile(current_file) - -- Get last or previous file in history that is not current_file - -- (self.ui.document.file, probided as current_file, might have - -- been removed from history) if not current_file then current_file = G_reader_settings:readSetting("lastfile") end - for i=1, #self.hist do + for _, v in ipairs(self.hist) do -- skip current document and deleted items kept in history - local file = self.hist[i].file - if file ~= current_file and lfs.attributes(file, "mode") == "file" then - return file + if v.file ~= current_file and lfs.attributes(v.file, "mode") == "file" then + return v.file end end end +--- Used in the BookShortcuts plugin. function ReadHistory:getFileByDirectory(directory, recursive) local real_path = realpath(directory) - for i=1, #self.hist do - local ipath = realpath(ffiutil.dirname(self.hist[i].file)) + for _, v in ipairs(self.hist) do + local ipath = realpath(ffiutil.dirname(v.file)) if ipath == real_path or (recursive and util.stringStartsWith(ipath, real_path)) then - return self.hist[i].file + return v.file end end end +function ReadHistory:updateItemByPath(old_path, new_path) + local index = self:getIndexByFile(old_path) + if index then + self.hist[index].file = new_path + self.hist[index].text = new_path:gsub(".*/", "") + self.hist[index].callback = function() + selectCallback(new_path) + end + self:_flush() + self:reload(true) + end + if G_reader_settings:readSetting("lastfile") == old_path then + G_reader_settings:saveSetting("lastfile", new_path) + end + self:ensureLastFile() +end + +--- Updates the history list after deleting a file. function ReadHistory:fileDeleted(path) - if G_reader_settings:isTrue("autoremove_deleted_items_from_history") then - self:removeItemByPath(path) - else - -- Make it dimed - for i=1, #self.hist do - if self.hist[i].file == path then - self.hist[i].dim = true - break - end + local index = self:getIndexByFile(path) + if index then + if G_reader_settings:isTrue("autoremove_deleted_items_from_history") then + self:removeItem(self.hist[index], index) + else + self.hist[index].dim = true + self:ensureLastFile() end - self:ensureLastFile() end end +--- Removes the history item if the document settings has been reset. function ReadHistory:fileSettingsPurged(path) if G_reader_settings:isTrue("autoremove_deleted_items_from_history") then -- Also remove it from history on purge when that setting is enabled @@ -226,81 +250,76 @@ function ReadHistory:fileSettingsPurged(path) end end +--- Checks the history list for deleted files and removes history items respectively. function ReadHistory:clearMissing() - for i = #self.hist, 1, -1 do - if self.hist[i].file == nil or lfs.attributes(self.hist[i].file, "mode") ~= "file" then - self:removeItem(self.hist[i], i) + local history_updated + for i, v in ipairs(self.hist) do + if v.file == nil or lfs.attributes(v.file, "mode") ~= "file" then + self:removeItem(v, i, true) -- no flush + history_updated = true end end - self:ensureLastFile() -end - -function ReadHistory:removeItemByPath(path) - for i = #self.hist, 1, -1 do - if self.hist[i].file == path then - self:removeItem(self.hist[i]) - break - end + if history_updated then + self:_flush() + self:ensureLastFile() end - self:ensureLastFile() end -function ReadHistory:updateItemByPath(old_path, new_path) - for i = #self.hist, 1, -1 do - if self.hist[i].file == old_path then - self.hist[i].file = new_path - self.hist[i].text = new_path:gsub(".*/", "") - self:_flush() - self:reload(true) - self.hist[i].callback = function() - selectCallback(new_path) - end - break - end - end - if G_reader_settings:readSetting("lastfile") == old_path then - G_reader_settings:saveSetting("lastfile", new_path) +function ReadHistory:removeItemByPath(path) + local index = self:getIndexByFile(path) + if index then + self:removeItem(self.hist[index], index) end - self:ensureLastFile() end -function ReadHistory:removeItem(item, idx) - table.remove(self.hist, item.index or idx) +function ReadHistory:removeItem(item, idx, no_flush) + local index = idx or self:getIndexByTime(item.time, item.file:gsub(".*/", "")) + table.remove(self.hist, index) os.remove(DocSettings:getHistoryPath(item.file)) - self:_indexing(item.index or idx) - self:_flush() - self:ensureLastFile() + if not no_flush then + self:_flush() + self:ensureLastFile() + end end -function ReadHistory:addItem(file, ts) +--- Adds new item (last opened document) to the top of the history list. +-- If item time (ts) is passed, add item to the history list at this time position. +function ReadHistory:addItem(file, ts, no_flash) if file ~= nil and lfs.attributes(file, "mode") == "file" then + local index = self:getIndexByFile(realpath(file)) + if ts and index and self.hist[index].time == ts then + return -- this legacy item is in the history already + end local now = ts or os.time() - table.insert(self.hist, 1, buildEntry(now, file)) - --- @todo (zijiehe): We do not need to sort if we can use binary insert and - -- binary search. - -- util.execute("/bin/touch", "-a", file) - -- This emulates `touch -a` in LuaFileSystem's API, since it may be absent (Android) - -- or provided by busybox, which doesn't support the `-a` flag. local mtime = lfs.attributes(file, "modification") lfs.touch(file, now, mtime) - self:_sort() - self:_reduce() - self:_flush() - G_reader_settings:saveSetting("lastfile", file) + if index == 1 and not ts then -- last book + self.hist[1].time = now + else -- old or new book + if index then -- old book + table.remove(self.hist, index) + end + index = ts and self:getIndexByTime(ts, file:gsub(".*/", "")) or 1 + table.insert(self.hist, index, buildEntry(now, file)) + end + if not no_flash then + self:_reduce() + self:_flush() + self:ensureLastFile() + end + return true -- used while adding legacy items end end ---- Reloads history from history_file. --- @treturn boolean true if history_file has been updated and reload happened. +--- Reloads history from history_file and legacy history folder. function ReadHistory:reload(force_read) if self:_read(force_read) then self:_readLegacyHistory() - self:_sort() + if G_reader_settings:isTrue("autoremove_deleted_items_from_history") then + self:clearMissing() + end self:_reduce() - return true end - - return false end ReadHistory:_init() diff --git a/spec/unit/readhistory_spec.lua b/spec/unit/readhistory_spec.lua index 9c7f29457..3707dab4b 100644 --- a/spec/unit/readhistory_spec.lua +++ b/spec/unit/readhistory_spec.lua @@ -43,7 +43,6 @@ describe("ReadHistory module", function() local function assert_item_is(h, i, name, fileRemoved) assert.is.same(name, h.hist[i].text) - assert.is.same(i, h.hist[i].index) assert.is.same(joinPath(realpath(test_data_dir()), name), h.hist[i].file) if fileRemoved then assert.is_nil(realpath(test_file(name))) @@ -260,28 +259,6 @@ describe("ReadHistory module", function() rm(test_file("e")) end) - it("should remove duplicate entry", function() - rm(file("history.lua")) - touch(test_file("a")) - touch(test_file("b")) - local h = reload() - now = now + 61 - h:addItem(test_file("b"), now) - now = now + 61 - h:addItem(test_file("b"), now) - touch(legacy_history_file("a")) - now = now + 61 - h:addItem(test_file("a"), now) -- ensure a is before b - h = reload() - assert.is.same(2, #h.hist) - assert_item_is(h, 1, "a") - assert_item_is(h, 2, "b") - - rm(legacy_history_file("a")) - rm(test_file("a")) - rm(test_file("b")) - end) - it("should reduce the total count", function() local function to_file(i) return test_file(string.format("%04d", i))