netdoc: Make versions smaller in GenericRouterStatus

When the version is a Tor version, we can just parse it; otherwise,
we can intern it.  This shrinks GenericRouterStatus and avoids a lot
of extra help allocations.

crates/tor-netdoc/src/doc/netstatus/rs.rs

@@ -12,7 +12,10 @@ mod ns;
 use super::{NetstatusKwd, RelayFlags, RelayWeight};
 use crate::parse::parser::Section;
 use crate::types::misc::*;
-use crate::{ParseErrorKind as EK, Result};
+use crate::types::version::TorVersion;
+use crate::util::intern::InternCache;
+use crate::{Error, ParseErrorKind as EK, Result};
+use std::sync::Arc;
 use std::{net, time};
 
 use tor_llcrypto::pk::rsa::RsaIdentity;
@@ -39,7 +42,7 @@ struct GenericRouterStatus<D> {
     /// Flags applied by the authorities to this relay.
     flags: RelayFlags,
     /// Version of the software that this relay is running.
-    version: Option<String>,
+    version: Option<Version>,
     /// List of subprotocol versions supported by this relay.
     protos: Protocols,
     /// Information about how to weight this relay when choosing a
@@ -47,6 +50,41 @@ struct GenericRouterStatus<D> {
     weight: RelayWeight,
 }
 
+/// A version as presented in a router status.
+///
+/// This can either be a parsed Tor version, or an unparsed string.
+//
+// TODO: This might want to merge, at some point, with routerdesc::RelayPlatform.
+#[derive(Clone, Debug, Eq, PartialEq, Hash, derive_more::Display)]
+#[non_exhaustive]
+pub enum Version {
+    /// A Tor version
+    Tor(TorVersion),
+    /// A string we couldn't parse.
+    Other(Arc<str>),
+}
+
+/// A cache of unparsable version strings.
+///
+/// We use this because we expect there not to be very many distinct versions of
+/// relay software in existence.
+static OTHER_VERSION_CACHE: InternCache<str> = InternCache::new();
+
+impl std::str::FromStr for Version {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self> {
+        let mut elts = s.splitn(3, ' ');
+        if elts.next() == Some("Tor") {
+            if let Some(Ok(v)) = elts.next().map(str::parse) {
+                return Ok(Version::Tor(v));
+            }
+        }
+
+        Ok(Version::Other(OTHER_VERSION_CACHE.intern(s)))
+    }
+}
+
 /// Implement a set of accessor functions on a given routerstatus type.
 // TODO: These methods should probably become, in whole or in part,
 // methods on the RouterStatus trait.
@@ -78,8 +116,8 @@ macro_rules! implement_accessors {
                 &self.rs.flags
             }
             /// Return the version of this routerstatus.
-            pub fn version(&self) -> &Option<String> {
-                &self.rs.version
+            pub fn version(&self) -> Option<&crate::doc::netstatus::rs::Version> {
+                self.rs.version.as_ref()
             }
             /// Return true if the ed25519 identity on this relay reflects a
             /// true consensus among the authorities.
@@ -168,7 +206,7 @@ where
         let flags = RelayFlags::from_item(sec.required(RS_S)?)?;
 
         // V line
-        let version = sec.maybe(RS_V).args_as_str().map(str::to_string);
+        let version = sec.maybe(RS_V).args_as_str().map(str::parse).transpose()?;
 
         // PR line
         let protos = {

crates/tor-netdoc/src/doc/netstatus/rs/build.rs

@@ -135,13 +135,14 @@ impl<D: Clone> RouterStatusBuilder<D> {
             .ok_or(Error::CannotBuild("Missing protocols"))?
             .clone();
         let weight = self.weight.unwrap_or(RelayWeight::Unmeasured(0));
+        let version = self.version.as_deref().map(str::parse).transpose()?;
 
         Ok(GenericRouterStatus {
             nickname,
             identity,
             addrs: self.addrs.clone(),
             doc_digest,
-            version: self.version.clone(),
+            version,
             protos,
             flags: self.flags,
             weight,

crates/tor-netdoc/src/types/version.rs

@@ -50,7 +50,7 @@ use crate::{ParseErrorKind as EK, Pos};
 /// a release candidate (rc), or stable.
 ///
 /// We accept unrecognized tags, and store them as "Other".
-#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[repr(u8)]
 enum TorVerStatus {
     /// An unknown release status
@@ -80,7 +80,7 @@ impl TorVerStatus {
 }
 
 /// A parsed Tor version number.
-#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
+#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub struct TorVersion {
     /// Major version number.  This has been zero since Tor was created.
     major: u8,

crates/tor-netdoc/src/util/intern.rs

@@ -15,12 +15,12 @@ use weak_table::WeakHashSet;
 /// It's "weak" because it only holds weak references to its objects;
 /// once every strong reference is gone, the object is unallocated.
 /// Later, the hash entry is (lazily) removed.
-pub(crate) struct InternCache<T> {
+pub(crate) struct InternCache<T: ?Sized> {
     /// Underlying hashset for interned objects
     cache: OnceCell<Mutex<WeakHashSet<Weak<T>>>>,
 }
 
-impl<T> InternCache<T> {
+impl<T: ?Sized> InternCache<T> {
     /// Create a new, empty, InternCache.
     pub(crate) const fn new() -> Self {
         InternCache {
@@ -29,13 +29,15 @@ impl<T> InternCache<T> {
     }
 }
 
-impl<T: Eq + Hash> InternCache<T> {
+impl<T: Eq + Hash + ?Sized> InternCache<T> {
     /// Helper: initialize the cache if needed, then lock it.
     fn cache(&self) -> MutexGuard<'_, WeakHashSet<Weak<T>>> {
         let cache = self.cache.get_or_init(|| Mutex::new(WeakHashSet::new()));
         cache.lock().expect("Poisoned lock lock for cache")
     }
+}
 
+impl<T: Eq + Hash> InternCache<T> {
     /// Intern a given value into this cache.
     ///
     /// If `value` is already stored in this cache, we return a
@@ -52,3 +54,17 @@ impl<T: Eq + Hash> InternCache<T> {
         }
     }
 }
+
+impl InternCache<str> {
+    /// Intern a string slice into this cache.
+    pub(crate) fn intern(&self, value: &str) -> Arc<str> {
+        let mut cache = self.cache();
+        if let Some(arc) = cache.get(value) {
+            arc
+        } else {
+            let arc = value.into();
+            cache.insert(Arc::clone(&arc));
+            arc
+        }
+    }
+}

doc/semver_status.md

@@ -45,3 +45,5 @@ tor-netdoc:
   consensus.
 
 
+tor-netdoc:
+  api-break: changed the return type of GenericRouterStatus::version()