From 869ac21f7bdcbbd8b094047f823101623f9b1a19 Mon Sep 17 00:00:00 2001
From: Pekka Enberg <penberg@iki.fi>
Date: Thu, 1 Aug 2019 18:06:40 +0300
Subject: [PATCH] clif-backend: Eliminate FunctionEnvironment construction in
 feed_event()

The feed_event() function is called for every wasm binary instruction.
Let's optimize it by storing FunctionEnvironment object in
CraneliftFunctionCodeGenerator, rather than constructing it for every
feed_event() invocation.

This change reduces the time to run "ngix compile" benchmark by 68%:

Before:

  nginx compile           time:   [1.4152 s 1.4186 s 1.4222 s]
  Found 1 outliers among 10 measurements (10.00%)
    1 (10.00%) high mild

After:

  nginx compile           time:   [447.76 ms 448.32 ms 448.80 ms]
                          change: [-68.542% -68.440% -68.352%] (p = 0.00 < 0.05)
                          Performance has improved.

I assume some of the clone() calls are very expensive (Vec::clone(),
likely). I did see libc malloc()/free() high up in "perf top" profiles,
which are eliminted by this change.

I also looked into eliminating FunctionBuilder construction from
feed_event(). That turns out to be painful on lifetime rules because it
borrows bunch of other objects, so I am leaving it for someone who knows
the code better than I do.
---
 lib/clif-backend/src/code.rs | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/clif-backend/src/code.rs b/lib/clif-backend/src/code.rs
index 947ea88e2..359b9f3e4 100644
--- a/lib/clif-backend/src/code.rs
+++ b/lib/clif-backend/src/code.rs
@@ -89,10 +89,13 @@ impl ModuleCodeGenerator<CraneliftFunctionCodeGenerator, Caller, CodegenError>
             func,
             func_translator,
             next_local: 0,
-            clif_signatures: self.clif_signatures.clone(),
             module_info: Arc::clone(&module_info),
-            target_config: self.isa.frontend_config().clone(),
             position: Position::default(),
+            func_env: FunctionEnvironment {
+                module_info: Arc::clone(&module_info),
+                target_config: self.isa.frontend_config().clone(),
+                clif_signatures: self.clif_signatures.clone(),
+            },
         };
 
         debug_assert_eq!(func_env.func.dfg.num_ebbs(), 0, "Function must be empty");
@@ -391,10 +394,9 @@ pub struct CraneliftFunctionCodeGenerator {
     func: Function,
     func_translator: FuncTranslator,
     next_local: usize,
-    pub clif_signatures: Map<SigIndex, ir::Signature>,
     module_info: Arc<RwLock<ModuleInfo>>,
-    target_config: isa::TargetFrontendConfig,
     position: Position,
+    func_env: FunctionEnvironment,
 }
 
 pub struct FunctionEnvironment {
@@ -1138,11 +1140,6 @@ impl FunctionCodeGenerator<CodegenError> for CraneliftFunctionCodeGenerator {
         //let builder = self.builder.as_mut().unwrap();
         //let func_environment = FuncEnv::new();
         //let state = TranslationState::new();
-        let mut function_environment = FunctionEnvironment {
-            module_info: Arc::clone(&self.module_info),
-            target_config: self.target_config.clone(),
-            clif_signatures: self.clif_signatures.clone(),
-        };
 
         if self.func_translator.state.control_stack.is_empty() {
             return Ok(());
@@ -1154,7 +1151,7 @@ impl FunctionCodeGenerator<CodegenError> for CraneliftFunctionCodeGenerator {
             &mut self.position,
         );
         let state = &mut self.func_translator.state;
-        translate_operator(op, &mut builder, state, &mut function_environment)?;
+        translate_operator(op, &mut builder, state, &mut self.func_env)?;
         Ok(())
     }