feat: Add new DefaultMachineRunner trait for initialization work (#470)

* feat: Add new DefaultMachineRunner trait for initialization work

Previously, all different machines in ckb-vm might use slightly
different initialization process. This commit introduces a new
DefaultMachineRunner trait, so the same initialization & usage workflow
can be used for all types implementing this trait.

This is a cherry-pick of #469
to the develop branch. 2 more changes are applied on the develop branch:

* Since AsmCoreMachine is now constant, we can simply use AsmCoreMachine
instead of `Box<AsmCoreMachine>`.
* #343 was mostly reverted. Upon
more thinking, I think the change earlier does not make sense now. It's better
we provide unified initialization for the whole machine, not just for the memory
part.

Due to the new InstDecoder / TraceDecoder structure introduced in develop
branch, more changes are required in the develop branch than 0.24.x releases.
Some of those changes include:

* Assembly machines now require TraceDecoder instead of InstDecoder. This means
we cannot use DefaultMachineBuilder to build DefaultMachine structures for both
types of VMs. The assembly machine types require more constraints. Given this
consideration, DefaultMachineBuilder is removed in this commit. 2 new types
RustDefaultMachineBuilder and AsmDefaultMachineBuilder are added for 2 types of
VMs respectively.
* A new set of types that begin with `Abstract`, such as AbstractDefaultMachineBuilder,
AbstractAsmMachine, AbstractTraceMachine are introduced. One is only expected to
use those types when one wants to tweak the inst/trace decoder used. Otherwise,
one is safe to pick AsmMachine/AsmDefaultMachineBuilder or TraceMachine/RustDefaultMachineBuilder.

So the current workflow is like following:

1. Pick a type implementing DefaultMachineRunner traits. Most likely, one would
pick either AsmMachine or TraceMachine.
2. Instantiate a core machine using DefaultMachineRunner::Inner::new.
3. Based on the type picked in step 1, use AsmDefaultMachineBuilder or
RustDefaultMachineBuilder to build a default machine with possible instruction
cycle func, syscalls and debugger plugin.
4. Finally, use DefaultMachineRunner::new to create the final runnable machine
from the default machine.

For advanced usages where the decoder is customized, one can use AbstractAsmMachine
or AbstractTraceMachine type. Then use AbstractDefaultMachineBuilder
to build the default machine. And finally create the runnable machine. All those
types accept an additional InstDecoder / TraceDecoder trait impl that allows one
to customize the decoder for optimizations.

* chore: Remove unneeded PhantomData

Author: xxuejie

.github/workflows/develop.yml

@@ -89,7 +89,7 @@ jobs:
           ln -snf .. ckb-vm-test-suite/ckb-vm
           docker run --rm -v `pwd`:/code nervos/ckb-riscv-gnu-toolchain:bionic-20210804 cp -r /riscv /code/riscv
           cd ckb-vm-test-suite
-          git checkout 898edc351eeb4de974ca4f0ff8d1e4943a95aecb
+          git checkout 2be7dcb0fda7ab41932813632fd1904e8cecf1f0
           git submodule update --init --recursive
           RISCV=`pwd`/../riscv ./test.sh
 
@@ -161,7 +161,7 @@ jobs:
           ln -snf .. ckb-vm-test-suite/ckb-vm
           docker run --rm -v `pwd`:/code nervos/ckb-riscv-gnu-toolchain:bionic-20210804 cp -r /riscv /code/riscv
           cd ckb-vm-test-suite
-          git checkout 898edc351eeb4de974ca4f0ff8d1e4943a95aecb
+          git checkout 2be7dcb0fda7ab41932813632fd1904e8cecf1f0
           git submodule update --init --recursive
           RISCV=`pwd`/../riscv ./test.sh --build-only
           cd ..

benches/vm_benchmark.rs

@@ -4,13 +4,13 @@ extern crate criterion;
 use bytes::Bytes;
 #[cfg(has_asm)]
 use ckb_vm::{
-    decoder::build_decoder,
+    decoder::{DefaultDecoder, InstDecoder},
     machine::{
         asm::{
             traces::{MemoizedDynamicTraceDecoder, MemoizedFixedTraceDecoder},
-            AsmCoreMachine, AsmMachine,
+            AbstractAsmMachine, AsmCoreMachine, AsmDefaultMachineBuilder, AsmMachine,
         },
-        DefaultMachineBuilder, VERSION0, VERSION2,
+        AbstractDefaultMachineBuilder, DefaultMachineRunner, SupportMachine, VERSION0, VERSION2,
     },
     ISA_B, ISA_IMC, ISA_MOP,
 };
@@ -45,7 +45,7 @@ fn asm_benchmark(c: &mut Criterion) {
         ].into_iter().map(|a| Ok(a.into()));
         b.iter(|| {
             let asm_core = AsmCoreMachine::new(ISA_IMC, VERSION0, u64::MAX);
-            let core = DefaultMachineBuilder::new(asm_core).build();
+            let core = AsmDefaultMachineBuilder::new(asm_core).build();
             let mut machine = AsmMachine::new(core);
             machine.load_program(&buffer, args.clone()).unwrap();
             machine.run().unwrap()
@@ -66,7 +66,7 @@ fn mop_benchmark(c: &mut Criterion) {
         ].into_iter().map(|a| Ok(a.into()));
         b.iter(|| {
             let asm_core = AsmCoreMachine::new(ISA_IMC | ISA_B | ISA_MOP, VERSION2, u64::MAX);
-            let core = DefaultMachineBuilder::<Box<AsmCoreMachine>>::new(asm_core)
+            let core = AsmDefaultMachineBuilder::new(asm_core)
                 .build();
             let mut machine = AsmMachine::new(core);
             machine.load_program(&buffer, args.clone()).unwrap();
@@ -88,19 +88,19 @@ fn mop_memoized_benchmark(c: &mut Criterion) {
             "foo",
             "bar",
         ].into_iter().map(|a| Ok(a.into()));
-        let mut decoder = MemoizedFixedTraceDecoder::new(build_decoder::<u64>(isa, version));
+        let mut decoder = MemoizedFixedTraceDecoder::new::<u64>(isa, version);
         let asm_core = AsmCoreMachine::new(isa, version, u64::MAX);
-        let core = DefaultMachineBuilder::<Box<AsmCoreMachine>>::new(asm_core)
+        let core = AbstractDefaultMachineBuilder::<_, MemoizedFixedTraceDecoder<DefaultDecoder>>::new(asm_core)
             .build();
-        let mut machine = AsmMachine::new(core);
+        let mut machine = AbstractAsmMachine::new(core);
         machine.load_program(&buffer, args.clone()).unwrap();
         machine.run_with_decoder(&mut decoder).unwrap();
 
         b.iter(|| {
             let asm_core = AsmCoreMachine::new(isa, version, u64::MAX);
-            let core = DefaultMachineBuilder::<Box<AsmCoreMachine>>::new(asm_core)
+            let core = AbstractDefaultMachineBuilder::<_, MemoizedFixedTraceDecoder<DefaultDecoder>>::new(asm_core)
                 .build();
-            let mut machine = AsmMachine::new(core);
+            let mut machine = AbstractAsmMachine::new(core);
             machine.load_program(&buffer, args.clone()).unwrap();
             decoder.clear_traces();
             machine.run_with_decoder(&mut decoder).unwrap()
@@ -121,19 +121,19 @@ fn mop_memoized_dynamic_benchmark(c: &mut Criterion) {
             "foo",
             "bar",
         ].into_iter().map(|a| Ok(a.into()));
-        let mut decoder = MemoizedDynamicTraceDecoder::new(build_decoder::<u64>(isa, version));
+        let mut decoder = MemoizedDynamicTraceDecoder::new::<u64>(isa, version);
         let asm_core = AsmCoreMachine::new(isa, version, u64::MAX);
-        let core = DefaultMachineBuilder::<Box<AsmCoreMachine>>::new(asm_core)
+        let core = AbstractDefaultMachineBuilder::<_, MemoizedDynamicTraceDecoder<DefaultDecoder>>::new(asm_core)
             .build();
-        let mut machine = AsmMachine::new(core);
+        let mut machine = AbstractAsmMachine::new(core);
         machine.load_program(&buffer, args.clone()).unwrap();
         machine.run_with_decoder(&mut decoder).unwrap();
 
         b.iter(|| {
             let asm_core = AsmCoreMachine::new(isa, version, u64::MAX);
-            let core = DefaultMachineBuilder::<Box<AsmCoreMachine>>::new(asm_core)
+            let core = AbstractDefaultMachineBuilder::<_, MemoizedDynamicTraceDecoder<DefaultDecoder>>::new(asm_core)
                 .build();
-            let mut machine = AsmMachine::new(core);
+            let mut machine = AbstractAsmMachine::new(core);
             machine.load_program(&buffer, args.clone()).unwrap();
             decoder.clear_traces();
             machine.run_with_decoder(&mut decoder).unwrap()

definitions/src/asm.rs

@@ -1,8 +1,5 @@
-use crate::{
-    instructions::Instruction, DEFAULT_MEMORY_SIZE, MEMORY_FRAMESIZE, MEMORY_FRAME_SHIFTS,
-    RISCV_GENERAL_REGISTER_NUMBER, RISCV_PAGESIZE,
-};
-use std::alloc::{alloc, alloc_zeroed, dealloc, Layout};
+use crate::{instructions::Instruction, RISCV_GENERAL_REGISTER_NUMBER};
+use std::alloc::{dealloc, Layout};
 
 // The number of trace items to keep
 pub const TRACE_SIZE: usize = 8192;
@@ -117,49 +114,6 @@ impl Drop for AsmCoreMachine {
 }
 
 impl AsmCoreMachine {
-    pub fn new(isa: u8, version: u32, max_cycles: u64) -> Box<AsmCoreMachine> {
-        Self::new_with_memory(isa, version, max_cycles, DEFAULT_MEMORY_SIZE)
-    }
-
-    pub fn new_with_memory(
-        isa: u8,
-        version: u32,
-        max_cycles: u64,
-        memory_size: usize,
-    ) -> Box<AsmCoreMachine> {
-        assert_ne!(memory_size, 0);
-        assert_eq!(memory_size % RISCV_PAGESIZE, 0);
-        assert_eq!(memory_size % (1 << MEMORY_FRAME_SHIFTS), 0);
-        let mut machine = unsafe {
-            let layout = Layout::new::<AsmCoreMachine>();
-            let raw_allocation = alloc_zeroed(layout) as *mut AsmCoreMachine;
-            Box::from_raw(raw_allocation)
-        };
-        machine.max_cycles = max_cycles;
-        if cfg!(feature = "enable-chaos-mode-by-default") {
-            machine.chaos_mode = 1;
-        }
-        machine.load_reservation_address = u64::MAX;
-        machine.version = version;
-        machine.isa = isa;
-
-        machine.memory_size = memory_size as u64;
-        machine.frames_size = (memory_size / MEMORY_FRAMESIZE) as u64;
-        machine.flags_size = (memory_size / RISCV_PAGESIZE) as u64;
-
-        machine.last_read_frame = u64::MAX;
-        machine.last_write_page = u64::MAX;
-
-        let memory_layout = Layout::array::<u8>(machine.memory_size as usize).unwrap();
-        machine.memory_ptr = unsafe { alloc(memory_layout) } as u64;
-        let flags_layout = Layout::array::<u8>(machine.flags_size as usize).unwrap();
-        machine.flags_ptr = unsafe { alloc_zeroed(flags_layout) } as u64;
-        let frames_layout = Layout::array::<u8>(machine.frames_size as usize).unwrap();
-        machine.frames_ptr = unsafe { alloc_zeroed(frames_layout) } as u64;
-
-        machine
-    }
-
     pub fn set_max_cycles(&mut self, cycles: u64) {
         self.max_cycles = cycles;
     }

definitions/src/generate_asm_constants.rs

@@ -11,6 +11,7 @@ use ckb_vm_definitions::{
     MEMORY_FRAMESIZE, MEMORY_FRAME_PAGE_SHIFTS, MEMORY_FRAME_SHIFTS, RISCV_PAGESIZE,
     RISCV_PAGE_SHIFTS,
 };
+use std::alloc::{alloc, Layout};
 use std::mem::{size_of, zeroed};
 
 macro_rules! print_inst_label {
@@ -129,7 +130,15 @@ fn main() {
     );
     println!();
 
-    let m: Box<AsmCoreMachine> = AsmCoreMachine::new(0, 0, 0);
+    // We don't need a fully initialized AsmCoreMachine, only a dummy
+    // structure here will do.
+    let m: Box<AsmCoreMachine> = unsafe {
+        let machine_size = std::mem::size_of::<AsmCoreMachine>();
+
+        let layout = Layout::array::<u8>(machine_size).unwrap();
+        let raw_allocation = alloc(layout) as *mut AsmCoreMachine;
+        Box::from_raw(raw_allocation)
+    };
     let m_address = &*m as *const AsmCoreMachine as usize;
     println!(
         "#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_REGISTERS {}",

examples/check_real_memory.rs

@@ -7,8 +7,8 @@ use std::process::{id, Command};
 #[cfg(has_asm)]
 use ckb_vm::{
     machine::{
-        asm::{AsmCoreMachine, AsmMachine},
-        DefaultMachineBuilder, VERSION0,
+        asm::{AsmCoreMachine, AsmDefaultMachineBuilder, AsmMachine},
+        DefaultMachineRunner, SupportMachine, VERSION0,
     },
     ISA_IMC,
 };
@@ -126,7 +126,7 @@ fn check_interpreter(memory_size: usize) -> Result<(), ()> {
         let result = run_with_memory::<u64, SparseMemory<u64>>(
             &Bytes::from(BIN_PATH_BUFFER),
             &vec![Bytes::from(BIN_NAME)],
-            SparseMemory::new_with_memory(memory_size),
+            memory_size,
         );
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), 0);
@@ -146,7 +146,7 @@ fn check_falt(memory_size: usize) -> Result<(), ()> {
         let result = run_with_memory::<u64, FlatMemory<u64>>(
             &Bytes::from(BIN_PATH_BUFFER),
             &vec![Bytes::from(BIN_NAME)],
-            FlatMemory::new_with_memory(memory_size),
+            memory_size,
         );
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), 0);
@@ -164,8 +164,13 @@ fn check_asm(memory_size: usize) -> Result<(), ()> {
     );
     println!("Base memory: {}", get_current_memory());
     for _ in 0..G_CHECK_LOOP {
-        let asm_core = AsmCoreMachine::new_with_memory(ISA_IMC, VERSION0, u64::MAX, memory_size);
-        let core = DefaultMachineBuilder::new(asm_core).build();
+        let asm_core = <AsmCoreMachine as SupportMachine>::new_with_memory(
+            ISA_IMC,
+            VERSION0,
+            u64::MAX,
+            memory_size,
+        );
+        let core = AsmDefaultMachineBuilder::new(asm_core).build();
         let mut machine = AsmMachine::new(core);
         machine
             .load_program(
@@ -191,8 +196,13 @@ fn check_asm_in_thread(memory_size: usize) -> Result<(), ()> {
     );
     println!("Base memory: {}", get_current_memory());
     for _ in 0..G_CHECK_LOOP {
-        let asm_core = AsmCoreMachine::new_with_memory(ISA_IMC, VERSION0, u64::MAX, memory_size);
-        let core = DefaultMachineBuilder::new(asm_core).build();
+        let asm_core = <AsmCoreMachine as SupportMachine>::new_with_memory(
+            ISA_IMC,
+            VERSION0,
+            u64::MAX,
+            memory_size,
+        );
+        let core = AsmDefaultMachineBuilder::new(asm_core).build();
         let mut machine = AsmMachine::new(core);
         machine
             .load_program(

examples/ckb_vm_runner.rs

@@ -1,6 +1,8 @@
 use ckb_vm::cost_model::estimate_cycles;
 use ckb_vm::registers::{A0, A7};
-use ckb_vm::{Bytes, CoreMachine, Memory, Register, SupportMachine, Syscalls};
+use ckb_vm::{
+    Bytes, CoreMachine, DefaultMachineRunner, Memory, Register, SupportMachine, Syscalls,
+};
 
 pub struct DebugSyscall {}
 
@@ -39,12 +41,12 @@ impl<Mac: SupportMachine> Syscalls<Mac> for DebugSyscall {
 
 #[cfg(has_asm)]
 fn main_asm(code: Bytes, args: Vec<Bytes>) -> Result<(), Box<dyn std::error::Error>> {
-    let asm_core = ckb_vm::machine::asm::AsmCoreMachine::new(
+    let asm_core = <ckb_vm::machine::asm::AsmCoreMachine as SupportMachine>::new(
         ckb_vm::ISA_IMC | ckb_vm::ISA_B | ckb_vm::ISA_MOP,
         ckb_vm::machine::VERSION2,
         u64::MAX,
     );
-    let core = ckb_vm::DefaultMachineBuilder::new(asm_core)
+    let core = ckb_vm::machine::asm::AsmDefaultMachineBuilder::new(asm_core)
         .instruction_cycle_func(Box::new(estimate_cycles))
         .syscall(Box::new(DebugSyscall {}))
         .build();
@@ -68,7 +70,7 @@ fn main_int(code: Bytes, args: Vec<Bytes>) -> Result<(), Box<dyn std::error::Err
         ckb_vm::machine::VERSION2,
         u64::MAX,
     );
-    let machine_builder = ckb_vm::DefaultMachineBuilder::new(core_machine)
+    let machine_builder = ckb_vm::RustDefaultMachineBuilder::new(core_machine)
         .instruction_cycle_func(Box::new(estimate_cycles));
     let mut machine = machine_builder.syscall(Box::new(DebugSyscall {})).build();
     machine.load_program(&code, args.into_iter().map(Ok))?;

src/decoder.rs

@@ -14,11 +14,12 @@ const RISCV_PAGESIZE_MASK: u64 = RISCV_PAGESIZE as u64 - 1;
 const INSTRUCTION_CACHE_SIZE: usize = 4096;
 
 pub trait InstDecoder {
+    fn new<R: Register>(isa: u8, version: u32) -> Self;
     fn decode<M: Memory>(&mut self, memory: &mut M, pc: u64) -> Result<Instruction, Error>;
     fn reset_instructions_cache(&mut self) -> Result<(), Error>;
 }
 
-pub struct Decoder {
+pub struct DefaultDecoder {
     factories: Vec<InstructionFactory>,
     mop: bool,
     version: u32,
@@ -30,9 +31,10 @@ pub struct Decoder {
     instructions_cache: Vec<(u64, u64)>,
 }
 
-impl Decoder {
-    pub fn new(mop: bool, version: u32) -> Decoder {
-        Decoder {
+impl DefaultDecoder {
+    /// Creates an empty decoder with no instruction factory
+    pub fn empty(mop: bool, version: u32) -> Self {
+        Self {
             factories: vec![],
             mop,
             version,
@@ -860,7 +862,21 @@ impl Decoder {
     }
 }
 
-impl InstDecoder for Decoder {
+impl InstDecoder for DefaultDecoder {
+    fn new<R: Register>(isa: u8, version: u32) -> Self {
+        let mut decoder = Self::empty(isa & ISA_MOP != 0, version);
+        decoder.add_instruction_factory(rvc::factory::<R>);
+        decoder.add_instruction_factory(i::factory::<R>);
+        decoder.add_instruction_factory(m::factory::<R>);
+        if isa & ISA_B != 0 {
+            decoder.add_instruction_factory(b::factory::<R>);
+        }
+        if isa & ISA_A != 0 {
+            decoder.add_instruction_factory(a::factory::<R>);
+        }
+        decoder
+    }
+
     fn decode<M: Memory>(&mut self, memory: &mut M, pc: u64) -> Result<Instruction, Error> {
         if self.mop {
             self.decode_mop(memory, pc)
@@ -874,17 +890,3 @@ impl InstDecoder for Decoder {
         Ok(())
     }
 }
-
-pub fn build_decoder<R: Register>(isa: u8, version: u32) -> Decoder {
-    let mut decoder = Decoder::new(isa & ISA_MOP != 0, version);
-    decoder.add_instruction_factory(rvc::factory::<R>);
-    decoder.add_instruction_factory(i::factory::<R>);
-    decoder.add_instruction_factory(m::factory::<R>);
-    if isa & ISA_B != 0 {
-        decoder.add_instruction_factory(b::factory::<R>);
-    }
-    if isa & ISA_A != 0 {
-        decoder.add_instruction_factory(a::factory::<R>);
-    }
-    decoder
-}