Continued refactoring of history

Use fstat() instead of lseek() to determine history file size.
Pass around the file_id instead of recomputing it.

This saves a few syscalls; no behavior change is expected.

src/env_universal_common.rs

@@ -175,7 +175,7 @@ pub fn sync(&mut self) -> (bool, Option<CallbackDataList>) {
         let rewrite = |old_file: &File,
                        tmp_file: &mut File|
          -> std::io::Result<PotentialUpdate<Option<UniversalReadUpdate>>> {
-            match self.load_from_file(old_file) {
+            match self.load_from_file(old_file, file_id_for_file(old_file)) {
                 Some(potential_update) => {
                     if potential_update.do_save {
                         let contents = Self::serialize_with_vars(&potential_update.data.new_vars);
@@ -364,8 +364,8 @@ fn load_from_path_narrow(&mut self) -> Option<CallbackDataList> {
         }
 
         FLOG!(uvar_file, "universal log reading from file");
-        match lock_and_load(&self.vars_path, |f| {
-            Ok(self.load_from_file(f).map(|update| update.data))
+        match lock_and_load(&self.vars_path, |f, file_id| {
+            Ok(self.load_from_file(f, file_id).map(|update| update.data))
         }) {
             Ok((
                 file_id,
@@ -402,9 +402,12 @@ fn load_from_path_narrow(&mut self) -> Option<CallbackDataList> {
     // IMPORTANT: Callers of this code assume that a return value of None means that the file id has
     // not changed. Do not return None in other situations without modifying the callers
     // accordingly. Otherwise, problems with self.ok_to_save are expected to occur.
-    fn load_from_file(&self, file: &File) -> Option<PotentialUpdate<UniversalReadUpdate>> {
+    fn load_from_file(
+        &self,
+        file: &File,
+        current_file_id: FileId,
+    ) -> Option<PotentialUpdate<UniversalReadUpdate>> {
         // Get the dev / inode.
-        let current_file_id = file_id_for_file(file);
         if current_file_id == self.last_read_file_id {
             FLOG!(uvar_file, "universal log sync elided based on fstat()");
             None

src/fs.rs

@@ -213,14 +213,13 @@ pub fn fsync(file: &File) -> std::io::Result<()> {
 /// If the file does not exist this function will return an error.
 pub fn lock_and_load<F, UserData>(path: &wstr, load: F) -> std::io::Result<(FileId, UserData)>
 where
-    F: Fn(&File) -> std::io::Result<UserData>,
+    F: Fn(&File, FileId) -> std::io::Result<UserData>,
 {
     match LockedFile::new(LockingMode::Shared, path) {
         Ok(locked_file) => {
-            return Ok((
-                file_id_for_file(locked_file.get()),
-                load(locked_file.get())?,
-            ));
+            let file_id = file_id_for_file(locked_file.get());
+            let user_data = load(locked_file.get(), file_id.clone())?;
+            return Ok((file_id, user_data));
         }
         Err(e) => {
             FLOGF!(
@@ -246,11 +245,11 @@ pub fn lock_and_load<F, UserData>(path: &wstr, load: F) -> std::io::Result<(File
     // Fallback implementation for situations where locking is unavailable.
     let max_attempts = 1000;
     for _ in 0..max_attempts {
-        let initial_file_id = file_id_for_path(path);
         // If we cannot open the file, there is nothing we can do,
         // so just return immediately.
         let file = wopen_cloexec(path, OFlag::O_RDONLY, Mode::empty())?;
-        let loaded_data = match load(&file) {
+        let initial_file_id = file_id_for_file(&file);
+        let loaded_data = match load(&file, initial_file_id.clone()) {
             Ok(update_data) => update_data,
             Err(_) => {
                 // Retry if load function failed. Because we do not hold a lock, this might be

src/history/file.rs

@@ -2,7 +2,7 @@
 
 use std::{
     fs::File,
-    io::{Read, Seek, SeekFrom, Write},
+    io::{Read, Write},
     ops::{Deref, DerefMut},
     os::fd::AsRawFd,
     time::{SystemTime, UNIX_EPOCH},
@@ -18,13 +18,14 @@
     common::wcs2bytes,
     flog::FLOG,
     path::{DirRemoteness, path_get_data_remoteness},
+    wutil::FileId,
 };
 
 /// History file types.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum HistoryFileType {
-    Fish2_0,
-    Fish1_x,
+    Fish2_0, // old format with just timestamp and item
+    Fish1_x, // YAML-style format
 }
 
 /// A type wrapping up the logic around mmap and munmap.
@@ -118,10 +119,10 @@ pub struct RawHistoryFile {
 }
 
 impl RawHistoryFile {
-    /// Construct a history file contents from a [`File`] reference.
-    pub fn create(mut history_file: &File) -> std::io::Result<Self> {
+    /// Construct a history file contents from a [`File`] reference and its file id.
+    pub fn create(history_file: &File, file_id: FileId) -> std::io::Result<Self> {
         // Check that the file is seekable, and its size.
-        let len: usize = match history_file.seek(SeekFrom::End(0))?.try_into() {
+        let len: usize = match file_id.size.try_into() {
             Ok(len) => len,
             Err(err) => {
                 return Err(std::io::Error::new(
@@ -138,7 +139,6 @@ pub fn create(mut history_file: &File) -> std::io::Result<Self> {
         let map_anon = |mut file: &File, len: usize| -> std::io::Result<MmapRegion> {
             let mut region = MmapRegion::map_anon(len)?;
             // If we mapped anonymous memory, we have to read from the file.
-            file.seek(SeekFrom::Start(0))?;
             file.read_exact(&mut region)?;
             Ok(region)
         };

src/history/history.rs

@@ -31,7 +31,6 @@
     fs::File,
     io::{BufRead, Read, Write},
     mem::MaybeUninit,
-    num::NonZeroUsize,
     ops::ControlFlow,
     sync::{Arc, Mutex, MutexGuard},
     time::{Duration, SystemTime, UNIX_EPOCH},
@@ -108,7 +107,7 @@ pub enum SearchDirection {
 
 /// When we rewrite the history, the number of items we keep.
 // FIXME: https://github.com/rust-lang/rust/issues/67441
-const HISTORY_SAVE_MAX: NonZeroUsize = NonZeroUsize::new(1024 * 256).unwrap();
+const HISTORY_SAVE_MAX: usize = 1024 * 256;
 
 /// Default buffer size for flushing to the history file.
 const HISTORY_OUTPUT_BUFFER_SIZE: usize = 64 * 1024;
@@ -519,11 +518,12 @@ fn rewrite_to_temporary_file(
         // We are reading FROM existing_file and writing TO dst
 
         // Make an LRU cache to save only the last N elements.
-        let mut lru = LruCache::new(HISTORY_SAVE_MAX);
+        let mut lru = LruCache::new(HISTORY_SAVE_MAX.try_into().unwrap());
 
         // Read in existing items (which may have changed out from underneath us, so don't trust our
         // old file contents).
-        if let Ok(local_file) = RawHistoryFile::create(existing_file) {
+        let file_id = file_id_for_file(existing_file);
+        if let Ok(local_file) = RawHistoryFile::create(existing_file, file_id) {
             for offset in local_file.offsets(None) {
                 // Try decoding an old item.
                 let Some(old_item) = local_file.decode_item(offset) else {