address feedback

gear-tech · Nov 6, 2023 · cd5184e · cd5184e
1 parent e174fa7
commit cd5184e
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 53 deletions.
diff --git a/core-backend/src/funcs.rs b/core-backend/src/funcs.rs
@@ -676,37 +676,42 @@ where
     }
 
     pub fn free_range(start: u32, end: u32) -> impl SysCall<Ext, i32> {
-        InfallibleSysCall::new(RuntimeCosts::Free, move |ctx: &mut CallerWrap<Ext>| {
-            if start > end {
-                log::trace!("Invalid range {start:?}:{end:?}");
-                return Err(UndefinedTerminationReason::Actor(
-                    ActorTerminationReason::Trap(TrapExplanation::Unknown),
-                ));
-            }
+        let page_count = start.abs_diff(end).saturating_add(1);
 
-            let err = |_| {
-                UndefinedTerminationReason::Actor(ActorTerminationReason::Trap(
-                    TrapExplanation::Unknown,
-                ))
-            };
+        InfallibleSysCall::new(
+            RuntimeCosts::FreeRange(page_count),
+            move |ctx: &mut CallerWrap<Ext>| {
+                if start > end {
+                    log::trace!("Invalid range {start:?}:{end:?}");
+                    return Err(UndefinedTerminationReason::Actor(
+                        ActorTerminationReason::Trap(TrapExplanation::Unknown),
+                    ));
+                }
 
-            let start = WasmPage::new(start).map_err(err)?;
-            let end = WasmPage::new(end).map_err(err)?;
+                let err = |_| {
+                    UndefinedTerminationReason::Actor(ActorTerminationReason::Trap(
+                        TrapExplanation::Unknown,
+                    ))
+                };
 
-            let res = ctx.ext_mut().free_range(start..=end);
-            let res = ctx.process_alloc_func_result(res)?;
+                let start = WasmPage::new(start).map_err(err)?;
+                let end = WasmPage::new(end).map_err(err)?;
 
-            match &res {
-                Ok(()) => {
-                    log::trace!("Free range {start:?}:{end:?}");
-                }
-                Err(err) => {
-                    log::trace!("Free failed: {err}");
-                }
-            };
+                let res = ctx.ext_mut().free_range(start..=end);
+                let res = ctx.process_alloc_func_result(res)?;
 
-            Ok(res.is_err() as i32)
-        })
+                match &res {
+                    Ok(()) => {
+                        log::trace!("Free range {start:?}:{end:?}");
+                    }
+                    Err(err) => {
+                        log::trace!("Free failed: {err}");
+                    }
+                };
+
+                Ok(res.is_err() as i32)
+            },
+        )
     }
 
     pub fn env_vars(vars_ver: u32, vars_ptr: u32) -> impl SysCall<Ext> {

diff --git a/core-processor/src/ext.rs b/core-processor/src/ext.rs
@@ -723,10 +723,6 @@ impl Externalities for Ext {
             .map_err(Into::into)
     }
 
-    fn free(&mut self, page: WasmPage) -> Result<(), Self::AllocError> {
-        self.free_range(page..=page)
-    }
-
     fn free_range(&mut self, range: RangeInclusive<WasmPage>) -> Result<(), Self::AllocError> {
         self.context
             .allocations_context

diff --git a/core/src/env.rs b/core/src/env.rs
@@ -193,7 +193,9 @@ pub trait Externalities {
     ) -> Result<WasmPage, Self::AllocError>;
 
     /// Free specific memory page
-    fn free(&mut self, page: WasmPage) -> Result<(), Self::AllocError>;
+    fn free(&mut self, page: WasmPage) -> Result<(), Self::AllocError> {
+        self.free_range(page..=page)
+    }
 
     /// Free specific memory range
     fn free_range(&mut self, range: RangeInclusive<WasmPage>) -> Result<(), Self::AllocError>;

diff --git a/core/src/memory.rs b/core/src/memory.rs
@@ -461,23 +461,14 @@ mod tests {
     #[test]
     fn free_fails() {
         let mut ctx = AllocationsContext::new(BTreeSet::default(), WasmPage(0), WasmPage(0));
-        assert_eq!(
-            ctx.free_range(WasmPage(1)..=WasmPage(1)),
-            Err(AllocError::InvalidFree(1))
-        );
+        assert_eq!(ctx.free(WasmPage(1)), Err(AllocError::InvalidFree(1)));
 
         let mut ctx = AllocationsContext::new(BTreeSet::default(), WasmPage(1), WasmPage(0));
-        assert_eq!(
-            ctx.free_range(WasmPage(0)..=WasmPage(0)),
-            Err(AllocError::InvalidFree(0))
-        );
+        assert_eq!(ctx.free(WasmPage(0)), Err(AllocError::InvalidFree(0)));
 
         let mut ctx =
             AllocationsContext::new(BTreeSet::from([WasmPage(0)]), WasmPage(1), WasmPage(1));
-        assert_eq!(
-            ctx.free_range(WasmPage(1)..=WasmPage(1)),
-            Err(AllocError::InvalidFree(1))
-        );
+        assert_eq!(ctx.free(WasmPage(1)), Err(AllocError::InvalidFree(1)));
 
         let mut ctx = AllocationsContext::new(
             BTreeSet::from([WasmPage(1), WasmPage(3)]),

diff --git a/pallets/gear/src/benchmarking/mod.rs b/pallets/gear/src/benchmarking/mod.rs
@@ -822,20 +822,28 @@ benchmarks! {
     }: {
         res.replace(run_process(exec));
     }
-
     verify {
         verify_process(res.unwrap());
     }
 
-
     free_range {
         let r in 0 .. API_BENCHMARK_BATCHES;
         let mut res = None;
-        let exec = Benches::<T>::free_range(r)?;
+        let exec = Benches::<T>::free_range(r, 1)?;
     }: {
         res.replace(run_process(exec));
     }
+    verify {
+        verify_process(res.unwrap());
+    }
 
+    free_range_per_page {
+        let r in 0 .. API_BENCHMARK_BATCHES;
+        let mut res = None;
+        let exec = Benches::<T>::free_range(1, 512)?;
+    }: {
+        res.replace(run_process(exec));
+    }
     verify {
         verify_process(res.unwrap());
     }

diff --git a/pallets/gear/src/benchmarking/syscalls.rs b/pallets/gear/src/benchmarking/syscalls.rs
@@ -262,17 +262,25 @@ where
         Self::prepare_handle(module, 0)
     }
 
-    pub fn free_range(r: u32) -> Result<Exec<T>, &'static str> {
-        assert!(r <= max_pages::<T>() as u32);
-
+    pub fn free_range(repetitions: u32, pages_per_call: u32) -> Result<Exec<T>, &'static str> {
         use Instruction::*;
         let mut instructions = vec![];
-        for _ in 0..API_BENCHMARK_BATCH_SIZE {
-            instructions.extend([I32Const(r as i32), Call(0), I32Const(-1)]);
+        for _ in 0..(API_BENCHMARK_BATCH_SIZE * repetitions) {
+            let n_pages = 512;
+            instructions.extend([I32Const(n_pages), Call(0), I32Const(-1)]);
             unreachable_condition(&mut instructions, I32Eq); // if alloc returns -1 then it's error
 
-            instructions.extend([I32Const(1), I32Const(r as i32 - 1), Call(1), I32Const(0)]);
-            unreachable_condition(&mut instructions, I32Ne); // if free_range returns 0 then it's error
+            // if pages_per_call is
+            for chunk in (1..=512)
+                .collect::<Vec<i32>>()
+                .chunks(pages_per_call as usize)
+            {
+                let start = *chunk.first().unwrap();
+                let end = *chunk.last().unwrap();
+
+                instructions.extend([I32Const(start), I32Const(end), Call(1), I32Const(0)]);
+                unreachable_condition(&mut instructions, I32Ne); // if free_range returns not 0 then it's error
+            }
         }
 
         let module = ModuleDefinition {