Remove branch prediction hack.

.github/workflows/benchmark.yml

@@ -3,6 +3,7 @@ on:
   push:
     branches:
       - master
+      - perf
 
 jobs:
   benchmark:

src/engine.rs

@@ -367,17 +367,4 @@ impl Engine {
     pub(crate) const fn is_debugger_registered(&self) -> bool {
         self.debugger_interface.is_some()
     }
-
-    /// Imitation of std::hints::black_box which requires nightly.
-    #[cfg(not(target_family = "wasm"))]
-    #[inline(never)]
-    pub(crate) fn black_box() -> usize {
-        unsafe { core::ptr::read_volatile(&0_usize as *const usize) }
-    }
-    /// Imitation of std::hints::black_box which requires nightly.
-    #[cfg(target_family = "wasm")]
-    #[inline(always)]
-    pub(crate) fn black_box() -> usize {
-        0
-    }
 }

src/eval/expr.rs

@@ -247,37 +247,9 @@ impl Engine {
 
         self.track_operation(global, expr.position())?;
 
-        // Function calls should account for a relatively larger portion of expressions because
-        // binary operators are also function calls.
-        if let Expr::FnCall(x, pos) = expr {
-            return self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos);
-        }
-
-        // Then variable access.
-        if let Expr::Variable(x, index, var_pos) = expr {
-            return if index.is_none() && x.0.is_none() && x.3 == KEYWORD_THIS {
-                this_ptr
-                    .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into())
-                    .cloned()
-            } else {
-                self.search_namespace(global, caches, scope, this_ptr, expr)
-                    .map(Target::take_or_clone)
-            };
-        }
-
-        // Then integer constants.
-        if let Expr::IntegerConstant(x, ..) = expr {
-            return Ok((*x).into());
-        }
-
-        // Stop merging branches here!
-        // We shouldn't lift out too many variants because, soon or later, the added comparisons
-        // will cost more than the mis-predicted `match` branch.
-        Self::black_box();
-
         match expr {
             // Constants
-            Expr::IntegerConstant(..) => unreachable!(),
+            Expr::IntegerConstant(x, ..) => Ok((*x).into()),
             Expr::StringConstant(x, ..) => Ok(x.clone().into()),
             Expr::BoolConstant(x, ..) => Ok((*x).into()),
             #[cfg(not(feature = "no_float"))]
@@ -286,7 +258,21 @@ impl Engine {
             Expr::Unit(..) => Ok(Dynamic::UNIT),
             Expr::DynamicConstant(x, ..) => Ok(x.as_ref().clone()),
 
-            // `... ${...} ...`
+            Expr::FnCall(x, pos) => {
+                self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos)
+            }
+
+            Expr::Variable(x, index, var_pos) => {
+                if index.is_none() && x.0.is_none() && x.3 == KEYWORD_THIS {
+                    this_ptr
+                        .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into())
+                        .cloned()
+                } else {
+                    self.search_namespace(global, caches, scope, this_ptr, expr)
+                        .map(Target::take_or_clone)
+                }
+            }
+
             Expr::InterpolatedString(x, _) => {
                 let mut concat = SmartString::new_const();
 

src/eval/stmt.rs

@@ -275,16 +275,31 @@ impl Engine {
 
         self.track_operation(global, stmt.position())?;
 
-        // Coded this way for better branch prediction.
+        match stmt {
-        // Popular branches are lifted out of the `match` statement into their own branches.
+            // No-op
+            Stmt::Noop(..) => Ok(Dynamic::UNIT),
 
-        // Function calls should account for a relatively larger portion of statements.
+            // Expression as statement
-        if let Stmt::FnCall(x, pos) = stmt {
+            Stmt::Expr(expr) => self
-            return self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos);
+                .eval_expr(global, caches, scope, this_ptr, expr)
+                .map(Dynamic::flatten),
+
+            // Block scope
+            Stmt::Block(statements, ..) => {
+                if statements.is_empty() {
+                    Ok(Dynamic::UNIT)
+                } else {
+                    self.eval_stmt_block(global, caches, scope, this_ptr, statements, true)
+                }
             }
 
-        // Then assignments.
+            // Function call
-        if let Stmt::Assignment(x, ..) = stmt {
+            Stmt::FnCall(x, pos) => {
+                self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos)
+            }
+
+            // Assignment
+            Stmt::Assignment(x, ..) => {
                 let (op_info, BinaryExpr { lhs, rhs }) = &**x;
 
                 if let Expr::Variable(x, ..) = lhs {
@@ -314,10 +329,7 @@ impl Engine {
                     self.track_operation(global, lhs.position())?;
 
                     self.eval_op_assignment(global, caches, op_info, lhs, &mut target, rhs_val)?;
-
+                } else {
-                return Ok(Dynamic::UNIT);
-            }
-
                     #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))]
                     {
                         let rhs_val = self
@@ -329,29 +341,31 @@ impl Engine {
 
                         // Must be either `var[index] op= val` or `var.prop op= val`.
                         // The return value of any op-assignment (should be `()`) is thrown away and not used.
-                let _ =
+                        let _ = match lhs {
-                    match lhs {
                             // name op= rhs (handled above)
                             Expr::Variable(..) => {
                                 unreachable!("Expr::Variable case is already handled")
                             }
                             // idx_lhs[idx_expr] op= rhs
                             #[cfg(not(feature = "no_index"))]
-                        Expr::Index(..) => self
+                            Expr::Index(..) => self.eval_dot_index_chain(
-                            .eval_dot_index_chain(global, caches, scope, this_ptr, lhs, _new_val),
+                                global, caches, scope, this_ptr, lhs, _new_val,
+                            ),
                             // dot_lhs.dot_rhs op= rhs
                             #[cfg(not(feature = "no_object"))]
-                        Expr::Dot(..) => self
+                            Expr::Dot(..) => self.eval_dot_index_chain(
-                            .eval_dot_index_chain(global, caches, scope, this_ptr, lhs, _new_val),
+                                global, caches, scope, this_ptr, lhs, _new_val,
+                            ),
                             _ => unreachable!("cannot assign to expression: {:?}", lhs),
                         }?;
-
-                return Ok(Dynamic::UNIT);
                     }
                 }
 
-        // Then variable definitions.
+                Ok(Dynamic::UNIT)
-        if let Stmt::Var(x, options, pos) = stmt {
+            }
+
+            // Variable definition
+            Stmt::Var(x, options, pos) => {
                 if !self.allow_shadowing() && scope.contains(&x.0) {
                     return Err(ERR::ErrorVariableExists(x.0.to_string(), *pos).into());
                 }
@@ -406,7 +420,9 @@ impl Engine {
                         && global.lib.iter().any(|m| !m.is_empty())
                     {
                         crate::func::locked_write(global.constants.get_or_insert_with(|| {
-                        crate::Shared::new(crate::Locked::new(std::collections::BTreeMap::new()))
+                            crate::Shared::new(
+                                crate::Locked::new(std::collections::BTreeMap::new()),
+                            )
                         }))
                         .insert(var_name.name.clone(), value.clone());
                     }
@@ -434,30 +450,7 @@ impl Engine {
                     scope.add_alias_by_index(scope.len() - 1, alias.as_str().into());
                 }
 
-            return Ok(Dynamic::UNIT);
-        }
-
-        // Stop merging branches here!
-        // We shouldn't lift out too many variants because, soon or later, the added comparisons
-        // will cost more than the mis-predicted `match` branch.
-        Self::black_box();
-
-        match stmt {
-            // No-op
-            Stmt::Noop(..) => Ok(Dynamic::UNIT),
-
-            // Expression as statement
-            Stmt::Expr(expr) => self
-                .eval_expr(global, caches, scope, this_ptr, expr)
-                .map(Dynamic::flatten),
-
-            // Block scope
-            Stmt::Block(statements, ..) => {
-                if statements.is_empty() {
                 Ok(Dynamic::UNIT)
-                } else {
-                    self.eval_stmt_block(global, caches, scope, this_ptr, statements, true)
-                }
             }
 
             // If statement