From 92a01c816d2676bce24bef63b21acf2e862af4e5 Mon Sep 17 00:00:00 2001 From: julian-seward1 Date: Thu, 12 Sep 2019 11:09:35 +0200 Subject: [PATCH] Minor speedup tuning for SecondaryMap (#1020) The `SecondaryMap` abstraction -- basically, resize-on-demand arrays with a default value -- is very hot in Cranelift. This small patch is the result of many profiling runs. It makes two changes: * `fn index_mut` is changed to be `#[inline(always)]`, based on profile data. * `fn index` and `fn index_mut` call `self.elems.resize()` directly, rather than via `self.resize()`. The point of this is not to improve performance. Rather, it ensures that the public functions for `SecondaryMap` do not call each other. When public interface functions call each other, it becomes difficult to interpret profiling results, because it's harder to see what fraction of costs for `SecondaryMap` as a whole come from outside the module, and what fraction is the result of "internal" calls to the external interface. The overall result, for wasm_lua_binarytrees, is a 1.4% reduction in instruction count for the compiler, and a 2.2% reduction in loads/stores. --- cranelift/entity/src/map.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cranelift/entity/src/map.rs b/cranelift/entity/src/map.rs index f46c307426..77fe38ce92 100644 --- a/cranelift/entity/src/map.rs +++ b/cranelift/entity/src/map.rs @@ -83,16 +83,19 @@ where } /// Get the element at `k` if it exists. + #[inline(always)] pub fn get(&self, k: K) -> Option<&V> { self.elems.get(k.index()) } /// Is this map completely empty? + #[inline(always)] pub fn is_empty(&self) -> bool { self.elems.is_empty() } /// Remove all entries from this map. + #[inline(always)] pub fn clear(&mut self) { self.elems.clear() } @@ -123,7 +126,6 @@ where } /// Resize the map to have `n` entries by adding default entries as needed. - #[inline] pub fn resize(&mut self, n: usize) { self.elems.resize(n, self.default.clone()); } @@ -139,8 +141,9 @@ where { type Output = V; + #[inline(always)] fn index(&self, k: K) -> &V { - self.get(k).unwrap_or(&self.default) + self.elems.get(k.index()).unwrap_or(&self.default) } } @@ -152,11 +155,11 @@ where K: EntityRef, V: Clone, { - #[inline] + #[inline(always)] fn index_mut(&mut self, k: K) -> &mut V { let i = k.index(); if i >= self.elems.len() { - self.resize(i + 1); + self.elems.resize(i + 1, self.default.clone()); } &mut self.elems[i] }