Consider making SizeLimit a trait rather than an enum

[dtolnay]

-pub struct Deserializer<R> {
+pub struct Deserializer<R, L> {
     reader: R,
-    size_limit: SizeLimit,
+    size_limit: L,
     read: u64
 }

This way the Infinite case would not be slowed down by all the branching in Deserializer::read_bytes:

match self.size_limit {
    SizeLimit::Infinite => Ok(()),
    SizeLimit::Bounded(x) if self.read <= x => Ok(()),
    SizeLimit::Bounded(_) => Err(ErrorKind::SizeLimit.into())
}

Something like:

trait SizeLimit {
    fn check(&self, read: u64) -> Result<()>;
}

struct Infinite;
impl SizeLimit for Infinite {
    #[inline]
    fn check(&self, _read: u64) -> Result<()> {
        Ok(())
    }
}

struct Bounded(u64);
impl SizeLimit for Bounded {
    fn check(&self, read: u64) -> Result<()> {
        if read <= self.0 {
            Ok(())
        } else {
            Err(ErrorKind::SizeLimit.into())
        }
    }
}

impl<R: Read, L: SizeLimit> Deserializer<R, L> {
    fn read_bytes(&mut self, count: u64) -> Result<()> {
        self.read += count;
        self.size_limit.check(self.read)
    }
}

The compiler may be able to generate faster code in the Infinite case knowing that no branching is happening.

[dtolnay]

A possible further advantage would be in the Infinite case, the size_of Deserializer drops from 40 bytes to 24 bytes (assuming reading from &[u8]) and in the Bounded case it drops to 32 bytes.

&[u8] // 16 bytes
bincode::SizeLimit // 16 bytes :(
u64 // 8 bytes

[TyOverby]

I was actually just thinking about this!

You could even go further and have the trait be something like:

trait SizeLimit {
    type Storage;
    fn create_storage() -> Self::Storage;
    fn add(&self, size: u64, storage: &mut Storage) -> Result<()>;
}

Then the Deserializer would look like

struct Deserializer<R: Read, L: SizeLimit> {
   reader: R,
   limit: L,
   storage: L::Storage,

This way, the implementation of Infinite would be

impl SizeLimit for Infinite {
    type Storage = ();
    fn add(&self, size: &u64, storage: &mut ()) -> Result<()> {}

[TyOverby]

^ Then sizeof(Deserializer<R, Infinite>) == sizeof(R)

[dtolnay]

A simpler / possibly faster alternative:

trait SizeLimit {
    fn add(&mut self, n: u64) -> Result<()>;
}

That way it doesn't need a pointer dereference for both self and storage.

[TyOverby]

but then implementors of SizeLimit would need to carry their own storage. This would make struct Bounded(u64) not work.

In addition, I don't think any implementors of SizeLimit would ever dereference self.

[dtolnay]

• struct Bounded(u64) would work:

  impl SizeLimit for Bounded {
      fn add(&mut self, n: u64) -> Result<()> {
          if self.0 >= n {
              self.0 -= n;
              Ok(())
          } else {
              Err(...)
          }
      }
  }

• Bounded would need to dereference self in order to find out what the bound is, right...? You were envisioning storing the bound in (*self).0 and storing the current pos in *storage.

[TyOverby]

Oh yeah, I suppose that does work. I was thinking of how I could expand this to also do structure size estimation, but I think I know how to deal with that now.