From 06b43952eccc23c068ce5f8f47fc4505db968f1b Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Jun 2021 13:49:03 -0700
Subject: [PATCH 1/2] limit

---
 src/ir/utils.h          |  6 ++++++
 src/passes/Inlining.cpp | 17 +++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/src/ir/utils.h b/src/ir/utils.h
index 78b546962d1..669c4aea53c 100644
--- a/src/ir/utils.h
+++ b/src/ir/utils.h
@@ -33,11 +33,17 @@ struct Measurer
 
   void visitExpression(Expression* curr) { size++; }
 
+  // Measure the number of expressions.
   static Index measure(Expression* tree) {
     Measurer measurer;
     measurer.walk(tree);
     return measurer.size;
   }
+
+  // A rough estimate of average binary size per expression. The number here is
+  // based on measurements on real-world (MVP) wasm files, on which observed
+  // ratios were 2.2 - 2.8.
+  static constexpr double BytesPerExpr = 2.5;
 };
 
 struct ExpressionAnalyzer {
diff --git a/src/passes/Inlining.cpp b/src/passes/Inlining.cpp
index 8601b340974..fc3412bf21c 100644
--- a/src/passes/Inlining.cpp
+++ b/src/passes/Inlining.cpp
@@ -420,6 +420,9 @@ struct Inlining : public Pass {
           continue;
         }
         Name inlinedName = inlinedFunction->name;
+        if (!canInlineInto(func->name, inlinedName)) {
+          continue;
+        }
 #ifdef INLINING_DEBUG
         std::cout << "inline " << inlinedName << " into " << func->name << '\n';
 #endif
@@ -446,6 +449,20 @@ struct Inlining : public Pass {
     // return whether we did any work
     return inlinedUses.size() > 0;
   }
+
+  // Checks if we are allowed to inline source into target.
+  bool canInlineInto(Name target, Name source) {
+    // We wish to avoid certain extremely-large sizes after inlining, as they
+    // may hit limits in VMs and/or slow down startup (measurements there
+    // indicate something like ~1 second to optimize a 100K function). See e.g.
+    // https://github.com/WebAssembly/binaryen/pull/3730#issuecomment-867939138
+    // https://github.com/emscripten-core/emscripten/issues/13899#issuecomment-825073344
+    auto combinedSize = infos[target].size + infos[source].size;
+    // Estimate the binary size from the number of instructions.
+    auto estimatedBinarySize = Measurer::BytesPerExpr * combinedSize;
+    const Index MaxCombinedBinarySize = 400 * 1024;
+    return estimatedBinarySize < MaxCombinedBinarySize;
+  }
 };
 
 Pass* createInliningPass() { return new Inlining(); }

From 4d55298236f73c54fc52578cd3d8188d4934a59d Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Wed, 30 Jun 2021 10:17:28 -0700
Subject: [PATCH 2/2] feedback

---
 src/passes/Inlining.cpp | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/passes/Inlining.cpp b/src/passes/Inlining.cpp
index fc3412bf21c..0e5d0234848 100644
--- a/src/passes/Inlining.cpp
+++ b/src/passes/Inlining.cpp
@@ -420,7 +420,7 @@ struct Inlining : public Pass {
           continue;
         }
         Name inlinedName = inlinedFunction->name;
-        if (!canInlineInto(func->name, inlinedName)) {
+        if (!isUnderSizeLimit(func->name, inlinedName)) {
           continue;
         }
 #ifdef INLINING_DEBUG
@@ -450,16 +450,20 @@ struct Inlining : public Pass {
     return inlinedUses.size() > 0;
   }
 
-  // Checks if we are allowed to inline source into target.
-  bool canInlineInto(Name target, Name source) {
-    // We wish to avoid certain extremely-large sizes after inlining, as they
-    // may hit limits in VMs and/or slow down startup (measurements there
-    // indicate something like ~1 second to optimize a 100K function). See e.g.
-    // https://github.com/WebAssembly/binaryen/pull/3730#issuecomment-867939138
-    // https://github.com/emscripten-core/emscripten/issues/13899#issuecomment-825073344
+  // Checks if the combined size of the code after inlining is under the
+  // absolute size limit. We have an absolute limit in order to avoid
+  // extremely-large sizes after inlining, as they may hit limits in VMs and/or
+  // slow down startup (measurements there indicate something like ~1 second to
+  // optimize a 100K function). See e.g.
+  // https://github.com/WebAssembly/binaryen/pull/3730#issuecomment-867939138
+  // https://github.com/emscripten-core/emscripten/issues/13899#issuecomment-825073344
+  bool isUnderSizeLimit(Name target, Name source) {
+    // Estimate the combined binary size from the number of instructions.
     auto combinedSize = infos[target].size + infos[source].size;
-    // Estimate the binary size from the number of instructions.
     auto estimatedBinarySize = Measurer::BytesPerExpr * combinedSize;
+    // The limit is arbitrary, but based on the links above. It is a very high
+    // value that should appear very rarely in practice (for example, it does
+    // not occur on the Emscripten benchmark suite of real-world codebases).
     const Index MaxCombinedBinarySize = 400 * 1024;
     return estimatedBinarySize < MaxCombinedBinarySize;
   }