Merge #266: heavily optimize the Value type

be8d437 value: special-case a bunch of primitive numeric types in from_bits (Andrew Poelstra)
674b272 value: from_bits: take a BitIter rather than an arbitrary bool-iter (Andrew Poelstra)
2691281 value: rewrite Display and CompactIter (Andrew Poelstra)
7cb40f5 value: rewrite module to use a byte vector as the backing store for Value (Andrew Poelstra)
d7a4215 types: return arcs from `FinalData` destructuring methods (Andrew Poelstra)
ee62770 value: replace as_left/as_right/as_product with to_* variants (Andrew Poelstra)
87992bc value: do some simple optimizations (Andrew Poelstra)
4196ab8 value: add regression test for type with nontrivial sum (Andrew Poelstra)
790b38a value: add tests for round-tripping compact bits (Andrew Poelstra)
8445f51 value: add test for padded/compact len (Andrew Poelstra)
eacf3a7 value: add benchmarks (Andrew Poelstra)

Pull request description:

  This PR has many commits but only one I would say is "big". This logic is all pretty-heavily covered by tests, and I added more tests, as well as benchmarks, so hopefully you don't need to review the fiddly bit-manipulation stuff too hard.

  It also leaves the `value` module in a somewhat messier state than when it started -- we have a new `ValueRef` type which duplicates some functionality of `Value` but isn't exposed in the public API; we now implement `DagLike` on `Value` itself rather than `&Value`, but using it is pretty slow compared to using it on `ValueRef`. The `Eq` and `Ord` traits are now implemented manually, but `Eq` requires a bit of computation and `Ord`, despite being implemented manually, still exposes a pretty-arbitrary ordering on values which have different types.

  So it is probably worthwhile to follow up on this with a PR that overhauls the public API and tries to provide a cleaner interface.

  **However**, this PR also introduces some massive speedups. It introduces new benchmarks and has their output in some commit messages. The highlights are as follows:

  Prior to this PR:
  ```
      benches::bench_value_create_64k               ... bench:   2,565,318.90 ns/iter (+/- 40,140.60)
      benches::bench_value_create_64k_compact       ... bench: 214,621,296.60 ns/iter (+/- 23,731,924.55)
      benches::bench_value_display_64k              ... bench:  17,526,983.30 ns/iter (+/- 339,118.95)
  ````

  With this PR:
  ```
      benches::bench_value_create_64k               ... bench:     875,039.00 ns/iter (+/- 5,007.70)
      benches::bench_value_create_64k_compact       ... bench:      45,258.88 ns/iter (+/- 169.22)
      benches::bench_value_display_64k              ... bench:   9,685,460.80 ns/iter (+/- 48,509.31)
  ```

  In other words, directly creating a value which is a 64 kilobyte (not kilobit) bitstring used to take 2.5ms; displaying took 17ms; and parsing it from a bit iterator took **214** milliseconds. Now these take 0.875ms, 9ms and 0.045ms, respectively. These are a 2.8, 1.8, and 4700x speedup respectively.

  The latter operation in particular, creating values from bitstrings and types, has been a major bottleneck in my fuzzer and is the motivation for this PR.

ACKs for top commit:
  uncomputable:
    ACK be8d437

Tree-SHA512: 315b19c6cb365ad0fdcb8ae734a8834059348b9b9862ef0e84de60db5b26f660f3d41689ec72ef60c6875320dca9952a86a3a7fdff9c02f67d8b32606cd30a90

Author: uncomputable

Cargo.toml

@@ -44,3 +44,6 @@ members = ["simpcli", "simplicity-sys", "fuzz"]
 # Should be manually/separately tested since it has a massive dep tree
 # and not follow MSRV
 exclude = ["jets-bench"]
+
+[lints.rust]
+unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)'] }

README.md

@@ -12,3 +12,22 @@ The MSRV of this crate is **1.63.0**.
 Some of the jet files in the library are auto-generated from Haskell code. These can be updated `update_jets.sh`. This requires the user has `cabal` and other necessary things that are required to build simplicity haskell. Instructions for those can be found in the simplicity repository.
 
 This script also checks that the internal vendored version of simplicity has the same git hash as of the version from which we are auto-generating the code. If this is not the case, the script will fail. This is because we only vendor minimal required C simplicity code and not the entire simplicity repo.
+
+# Benchmarking
+
+There are two sets of benchmarks in this codebase. First, there is the `jets-bench`
+sub-crate which uses criterion to collect statistics about jet performance. These
+benchmarks are specifically targeted at the C jets and are intended to estimate
+consensus costs.
+
+See `jets-bench/README.md` for information about running these benchmarks.
+
+The second set of benchmarks are benchmarks for this library's performance. These
+are used to track performance of this library itself. These can be run with
+
+```
+RUSTFLAGS=--cfg=bench cargo +nightly bench
+```
+
+The custom `cfg` flag is used to prevent nightly-only code from polluting ordinary
+code.

src/bit_encoding/bititer.rs

@@ -69,6 +69,17 @@ pub enum u2 {
     _3,
 }
 
+impl From<u2> for u8 {
+    fn from(s: u2) -> u8 {
+        match s {
+            u2::_0 => 0,
+            u2::_1 => 1,
+            u2::_2 => 2,
+            u2::_3 => 3,
+        }
+    }
+}
+
 /// Bitwise iterator formed from a wrapped bytewise iterator. Bytes are
 /// interpreted big-endian, i.e. MSB is returned first
 #[derive(Debug)]

src/lib.rs

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: CC0-1.0
 
+#![cfg_attr(bench, feature(test))]
 #![allow(
     // we use `bool` to represent bits and frequentely assert_eq against them
     clippy::bool_assert_comparison,
@@ -23,6 +24,9 @@ pub extern crate hashes;
 /// Re-export of hex crate
 pub extern crate hex;
 
+#[cfg(bench)]
+extern crate test;
+
 #[macro_use]
 mod macros;
 

src/node/mod.rs

@@ -190,14 +190,14 @@ pub trait CoreConstructible: Sized {
     /// The expression is minimized by using as many word jets as possible.
     fn scribe(ctx: &types::Context, value: &Value) -> Self {
         #[derive(Debug, Clone)]
-        enum Task<'a> {
-            Process(&'a Value),
+        enum Task {
+            Process(Value),
             MakeLeft,
             MakeRight,
             MakeProduct,
         }
 
-        let mut input = vec![Task::Process(value)];
+        let mut input = vec![Task::Process(value.shallow_clone())];
         let mut output = vec![];
         while let Some(top) = input.pop() {
             match top {
@@ -206,13 +206,13 @@ pub trait CoreConstructible: Sized {
                         output.push(Self::unit(ctx));
                     } else if let Some(word) = value.to_word() {
                         output.push(Self::const_word(ctx, word));
-                    } else if let Some(left) = value.as_left() {
+                    } else if let Some(left) = value.to_left() {
                         input.push(Task::MakeLeft);
                         input.push(Task::Process(left));
-                    } else if let Some(right) = value.as_right() {
+                    } else if let Some(right) = value.to_right() {
                         input.push(Task::MakeRight);
                         input.push(Task::Process(right));
-                    } else if let Some((left, right)) = value.as_product() {
+                    } else if let Some((left, right)) = value.to_product() {
                         input.push(Task::MakeProduct);
                         input.push(Task::Process(right));
                         input.push(Task::Process(left));

src/types/final_data.rs

@@ -232,17 +232,17 @@ impl Final {
     }
 
     /// Access the inner types of a sum type.
-    pub fn as_sum(&self) -> Option<(&Self, &Self)> {
+    pub fn as_sum(&self) -> Option<(&Arc<Self>, &Arc<Self>)> {
         match &self.bound {
-            CompleteBound::Sum(left, right) => Some((left.as_ref(), right.as_ref())),
+            CompleteBound::Sum(left, right) => Some((left, right)),
             _ => None,
         }
     }
 
     /// Access the inner types of a product type.
-    pub fn as_product(&self) -> Option<(&Self, &Self)> {
+    pub fn as_product(&self) -> Option<(&Arc<Self>, &Arc<Self>)> {
         match &self.bound {
-            CompleteBound::Product(left, right) => Some((left.as_ref(), right.as_ref())),
+            CompleteBound::Product(left, right) => Some((left, right)),
             _ => None,
         }
     }