Patchwork D11632: dirstate-v2: Separate Rust structs for Timestamp and PackedTimestamp

login
register
mail settings
Submitter phabricator
Date Oct. 12, 2021, 5:02 p.m.
Message ID <differential-rev-PHID-DREV-m6czf4co6odbihwr6vqj-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49957/
State Superseded
Headers show

Comments

phabricator - Oct. 12, 2021, 5:02 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  PackedTimestamp is now exclusively for dirstate-v2 serialization purpose.
  It contains unaligned big-endian integers. Timestamp is used everywhere else
  and contains native Rust integers.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D11632

AFFECTED FILES
  rust/hg-core/src/dirstate/entry.rs
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/on_disk.rs
  rust/hg-core/src/dirstate_tree/status.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-core/src/dirstate_tree/status.rs b/rust/hg-core/src/dirstate_tree/status.rs
--- a/rust/hg-core/src/dirstate_tree/status.rs
+++ b/rust/hg-core/src/dirstate_tree/status.rs
@@ -1,3 +1,4 @@ 
+use crate::dirstate::entry::Timestamp;
 use crate::dirstate::status::IgnoreFnType;
 use crate::dirstate_tree::dirstate_map::BorrowedPath;
 use crate::dirstate_tree::dirstate_map::ChildNodesRef;
@@ -5,7 +6,6 @@ 
 use crate::dirstate_tree::dirstate_map::NodeData;
 use crate::dirstate_tree::dirstate_map::NodeRef;
 use crate::dirstate_tree::on_disk::DirstateV2ParseError;
-use crate::dirstate_tree::on_disk::Timestamp;
 use crate::matchers::get_ignore_function;
 use crate::matchers::Matcher;
 use crate::utils::files::get_bytes_from_os_string;
@@ -182,7 +182,7 @@ 
     fn can_skip_fs_readdir(
         &self,
         directory_metadata: Option<&std::fs::Metadata>,
-        cached_directory_mtime: Option<&Timestamp>,
+        cached_directory_mtime: Option<Timestamp>,
     ) -> bool {
         if !self.options.list_unknown && !self.options.list_ignored {
             // All states that we care about listing have corresponding
@@ -200,7 +200,7 @@ 
                 if let Some(meta) = directory_metadata {
                     if let Ok(current_mtime) = meta.modified() {
                         let current_mtime = Timestamp::from(current_mtime);
-                        if current_mtime == *cached_mtime {
+                        if current_mtime == cached_mtime {
                             // The mtime of that directory has not changed
                             // since then, which means that the results of
                             // `read_dir` should also be unchanged.
@@ -222,7 +222,7 @@ 
         directory_hg_path: &BorrowedPath<'tree, 'on_disk>,
         directory_fs_path: &Path,
         directory_metadata: Option<&std::fs::Metadata>,
-        cached_directory_mtime: Option<&Timestamp>,
+        cached_directory_mtime: Option<Timestamp>,
         is_at_repo_root: bool,
     ) -> Result<bool, DirstateV2ParseError> {
         if self.can_skip_fs_readdir(directory_metadata, cached_directory_mtime)
@@ -468,7 +468,7 @@ 
                     // unlikely enough in practice.
                     let timestamp = directory_mtime.into();
                     let cached = dirstate_node.cached_directory_mtime();
-                    if cached != Some(&timestamp) {
+                    if cached != Some(timestamp) {
                         let hg_path = dirstate_node
                             .full_path_borrowed(self.dmap.on_disk)?
                             .detach_from_tree();
diff --git a/rust/hg-core/src/dirstate_tree/on_disk.rs b/rust/hg-core/src/dirstate_tree/on_disk.rs
--- a/rust/hg-core/src/dirstate_tree/on_disk.rs
+++ b/rust/hg-core/src/dirstate_tree/on_disk.rs
@@ -2,6 +2,7 @@ 
 //!
 //! See `mercurial/helptext/internals/dirstate-v2.txt`
 
+use crate::dirstate::Timestamp;
 use crate::dirstate_tree::dirstate_map::{self, DirstateMap, NodeRef};
 use crate::dirstate_tree::path_with_basename::WithBasename;
 use crate::errors::HgError;
@@ -15,7 +16,6 @@ 
 use format_bytes::format_bytes;
 use std::borrow::Cow;
 use std::convert::{TryFrom, TryInto};
-use std::time::{SystemTime, UNIX_EPOCH};
 
 /// Added at the start of `.hg/dirstate` when the "v2" format is used.
 /// This a redundant sanity check more than an actual "magic number" since
@@ -119,9 +119,9 @@ 
 }
 
 /// Duration since the Unix epoch
-#[derive(BytesCast, Copy, Clone, PartialEq)]
+#[derive(BytesCast, Copy, Clone)]
 #[repr(C)]
-pub(super) struct Timestamp {
+struct PackedTimestamp {
     seconds: I64Be,
 
     /// In `0 .. 1_000_000_000`.
@@ -316,14 +316,14 @@ 
     ) -> Result<dirstate_map::NodeData, DirstateV2ParseError> {
         if self.has_entry() {
             Ok(dirstate_map::NodeData::Entry(self.assume_entry()))
-        } else if let Some(&mtime) = self.cached_directory_mtime() {
+        } else if let Some(mtime) = self.cached_directory_mtime() {
             Ok(dirstate_map::NodeData::CachedDirectory { mtime })
         } else {
             Ok(dirstate_map::NodeData::None)
         }
     }
 
-    pub(super) fn cached_directory_mtime(&self) -> Option<&Timestamp> {
+    pub(super) fn cached_directory_mtime(&self) -> Option<Timestamp> {
         if self.flags.contains(Flags::HAS_MTIME) && !self.has_entry() {
             Some(self.data.as_timestamp())
         } else {
@@ -423,58 +423,23 @@ 
     }
 
     fn from_timestamp(timestamp: Timestamp) -> Self {
+        let packed = PackedTimestamp {
+            seconds: timestamp.seconds().into(),
+            nanoseconds: timestamp.nanoseconds().into(),
+        };
         // Safety: both types implement the `ByteCast` trait, so we could
         // safely use `as_bytes` and `from_bytes` to do this conversion. Using
         // `transmute` instead makes the compiler check that the two types
         // have the same size, which eliminates the error case of
         // `from_bytes`.
-        unsafe { std::mem::transmute::<Timestamp, Entry>(timestamp) }
+        unsafe { std::mem::transmute::<PackedTimestamp, Entry>(packed) }
     }
 
-    fn as_timestamp(&self) -> &Timestamp {
+    fn as_timestamp(self) -> Timestamp {
         // Safety: same as above in `from_timestamp`
-        unsafe { &*(self as *const Entry as *const Timestamp) }
-    }
-}
-
-impl Timestamp {
-    pub fn seconds(&self) -> i64 {
-        self.seconds.get()
-    }
-}
-
-impl From<SystemTime> for Timestamp {
-    fn from(system_time: SystemTime) -> Self {
-        // On Unix, `SystemTime` is a wrapper for the `timespec` C struct:
-        // https://www.gnu.org/software/libc/manual/html_node/Time-Types.html#index-struct-timespec
-        // We want to effectively access its fields, but the Rust standard
-        // library does not expose them. The best we can do is:
-        let (secs, nanos) = match system_time.duration_since(UNIX_EPOCH) {
-            Ok(duration) => {
-                (duration.as_secs() as i64, duration.subsec_nanos())
-            }
-            Err(error) => {
-                // `system_time` is before `UNIX_EPOCH`.
-                // We need to undo this algorithm:
-                // https://github.com/rust-lang/rust/blob/6bed1f0bc3cc50c10aab26d5f94b16a00776b8a5/library/std/src/sys/unix/time.rs#L40-L41
-                let negative = error.duration();
-                let negative_secs = negative.as_secs() as i64;
-                let negative_nanos = negative.subsec_nanos();
-                if negative_nanos == 0 {
-                    (-negative_secs, 0)
-                } else {
-                    // For example if `system_time` was 4.3 seconds before
-                    // the Unix epoch we get a Duration that represents
-                    // `(-4, -0.3)` but we want `(-5, +0.7)`:
-                    const NSEC_PER_SEC: u32 = 1_000_000_000;
-                    (-1 - negative_secs, NSEC_PER_SEC - negative_nanos)
-                }
-            }
-        };
-        Timestamp {
-            seconds: secs.into(),
-            nanoseconds: nanos.into(),
-        }
+        let packed =
+            unsafe { std::mem::transmute::<Entry, PackedTimestamp>(self) };
+        Timestamp::new(packed.seconds.get(), packed.nanoseconds.get())
     }
 }
 
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -330,9 +330,9 @@ 
 
     pub(super) fn cached_directory_mtime(
         &self,
-    ) -> Option<&'tree on_disk::Timestamp> {
+    ) -> Option<crate::dirstate::Timestamp> {
         match self {
-            NodeRef::InMemory(_path, node) => match &node.data {
+            NodeRef::InMemory(_path, node) => match node.data {
                 NodeData::CachedDirectory { mtime } => Some(mtime),
                 _ => None,
             },
@@ -376,7 +376,7 @@ 
 
 pub(super) enum NodeData {
     Entry(DirstateEntry),
-    CachedDirectory { mtime: on_disk::Timestamp },
+    CachedDirectory { mtime: crate::dirstate::Timestamp },
     None,
 }
 
diff --git a/rust/hg-core/src/dirstate/entry.rs b/rust/hg-core/src/dirstate/entry.rs
--- a/rust/hg-core/src/dirstate/entry.rs
+++ b/rust/hg-core/src/dirstate/entry.rs
@@ -1,6 +1,7 @@ 
 use crate::errors::HgError;
 use bitflags::bitflags;
 use std::convert::TryFrom;
+use std::time::{SystemTime, UNIX_EPOCH};
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum EntryState {
@@ -28,6 +29,73 @@ 
     }
 }
 
+#[derive(Copy, Clone, PartialEq)]
+pub struct Timestamp {
+    seconds: i64,
+
+    /// In `0 .. 1_000_000_000`.
+    ///
+    /// This timestamp is after `(seconds, 0)` by this many nanoseconds.
+    nanoseconds: u32,
+}
+
+impl Timestamp {
+    pub fn new(seconds: i64, nanoseconds: u32) -> Self {
+        Self {
+            seconds,
+            nanoseconds,
+        }
+    }
+
+    pub fn seconds(&self) -> i64 {
+        self.seconds
+    }
+
+    pub fn nanoseconds(&self) -> u32 {
+        self.nanoseconds
+    }
+}
+
+impl From<SystemTime> for Timestamp {
+    fn from(system_time: SystemTime) -> Self {
+        // On Unix, `SystemTime` is a wrapper for the `timespec` C struct:
+        // https://www.gnu.org/software/libc/manual/html_node/Time-Types.html#index-struct-timespec
+        // We want to effectively access its fields, but the Rust standard
+        // library does not expose them. The best we can do is:
+        let seconds;
+        let nanoseconds;
+        match system_time.duration_since(UNIX_EPOCH) {
+            Ok(duration) => {
+                seconds = duration.as_secs() as i64;
+                nanoseconds = duration.subsec_nanos();
+            }
+            Err(error) => {
+                // `system_time` is before `UNIX_EPOCH`.
+                // We need to undo this algorithm:
+                // https://github.com/rust-lang/rust/blob/6bed1f0bc3cc50c10aab26d5f94b16a00776b8a5/library/std/src/sys/unix/time.rs#L40-L41
+                let negative = error.duration();
+                let negative_secs = negative.as_secs() as i64;
+                let negative_nanos = negative.subsec_nanos();
+                if negative_nanos == 0 {
+                    seconds = -negative_secs;
+                    nanoseconds = 0;
+                } else {
+                    // For example if `system_time` was 4.3 seconds before
+                    // the Unix epoch we get a Duration that represents
+                    // `(-4, -0.3)` but we want `(-5, +0.7)`:
+                    const NSEC_PER_SEC: u32 = 1_000_000_000;
+                    seconds = -1 - negative_secs;
+                    nanoseconds = NSEC_PER_SEC - negative_nanos;
+                }
+            }
+        };
+        Self {
+            seconds,
+            nanoseconds,
+        }
+    }
+}
+
 pub const V1_RANGEMASK: i32 = 0x7FFFFFFF;
 
 pub const MTIME_UNSET: i32 = -1;